diff --git a/SECURITY.md b/SECURITY.md new file mode 100644 index 00000000..11c0833c --- /dev/null +++ b/SECURITY.md @@ -0,0 +1,55 @@ + +## Vulnerabilities in original engine + +This document points out vulnerabilities in original game engine and describes vectors of attack +to exploit them. + +This document is for educational purposes only to raise awareness (about learning how dangerous it is to run public Starbound +server on original engine), and pursues no goal of harming users of original engine. + +Experienced blackhats already could take sources and dig these invulnerabilities themselves, +since most of them are not buried anywhere deep in code. + +----------- + +### EntityDestroyPacket vulnerability + +When client sends EntityCreatePacket to WorldServer, it checks whenever received `entityId` is within +allowed range (range of IDs allocated specifically for that client). Same happens on EntityUpdateSetPacket. + +However, someone forgot to put the same check when receiving EntityDestroyPacket, hence +any client can remove ANY other entity inside world, including other PlayerEntitys'. + +On side note, original client makes sure it sends EntityDestroyPacket only for entities it owns. + +This attack require modified game client. + +----------- + +### Zip bomb in PacketSocket + +When packets are received on network socket, they are checked for not exceeding 16 MiB, +by reading packet length header. However, when receiving compressed packets, +only compressed size is checked against 16 MiB limit, and +they are uncompressed in one shot, without limiting uncompressed size. + +This vulnerability allows to make server quickly run out of memory by forging zip-bomb packet. + +This attack require modified game client. + +----------- + +### Client's ShipWorld size + +When joining server, client sends contents of `.shipworld` in form of chunk map +(Map with bytearray keys and bytearray values, which represent data stored inside BTreeDB). + +Server instances WorldServer with provided world chunks. The vulnerability lies within world's size. + +Original engine world's chunk map is always stored as tight 2D array of chunk (sector) pointers, +and pointer array is always fully preallocated when world is instanced. + +So client can forge custom shipworld, with 2^31 x 2^31 dimensions, which will instantly cause +server to consume at least 128 GiB of RAM when client connects. + +This attack does not require modified game client. diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/Main.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/Main.kt index 5525c62c..7a48fe04 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/Main.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/Main.kt @@ -103,8 +103,8 @@ fun main() { Starbound.mailboxInitialized.submit { val server = IntegratedStarboundServer(File("./")) - val world = ServerWorld.create(server, WorldGeometry(Vector2i(3000, 2000), true, false), LegacyWorldStorage.file(db)) - world.thread.start() + //val world = ServerWorld.create(server, WorldGeometry(Vector2i(3000, 2000), true, false), LegacyWorldStorage.file(db)) + //world.thread.start() //ply = PlayerEntity(client.world!!) @@ -113,7 +113,7 @@ fun main() { //client.world!!.parallax = Starbound.parallaxAccess["garden"] - client.connectToLocalServer(server.channels.createLocalChannel()) + //client.connectToLocalServer(server.channels.createLocalChannel()) //client.connectToRemoteServer(InetSocketAddress("127.0.0.1", 21025)) //client2.connectToLocalServer(server.channels.createLocalChannel(), UUID.randomUUID()) server.channels.createChannel(InetSocketAddress(21060)) diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/network/Connection.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/network/Connection.kt index 0440845b..74ab5bc8 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/network/Connection.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/network/Connection.kt @@ -39,7 +39,8 @@ abstract class Connection(val side: ConnectionSide, val type: ConnectionType) : set(value) { require(value in 1 .. ServerChannels.MAX_PLAYERS) { "Connection ID is out of range: $value" } field = value - entityIDRange = value * -65536 .. 65535 + val begin = value * -65536 + entityIDRange = begin .. begin + 65535 } var nickname: String = "" diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/network/PacketRegistry.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/network/PacketRegistry.kt index 8ec29183..3083252a 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/network/PacketRegistry.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/network/PacketRegistry.kt @@ -20,6 +20,7 @@ import ru.dbotthepony.kstarbound.client.network.packets.JoinWorldPacket import ru.dbotthepony.kstarbound.client.network.packets.SpawnWorldObjectPacket import ru.dbotthepony.kstarbound.network.packets.ClientContextUpdatePacket import ru.dbotthepony.kstarbound.network.packets.EntityCreatePacket +import ru.dbotthepony.kstarbound.network.packets.EntityDestroyPacket import ru.dbotthepony.kstarbound.network.packets.EntityUpdateSetPacket import ru.dbotthepony.kstarbound.network.packets.PingPacket import ru.dbotthepony.kstarbound.network.packets.PongPacket @@ -440,7 +441,7 @@ class PacketRegistry(val isLegacy: Boolean) { // Packets sent bidirectionally between world client and world server LEGACY.add(::EntityCreatePacket) LEGACY.add(EntityUpdateSetPacket::read) - LEGACY.skip("EntityDestroy") + LEGACY.add(::EntityDestroyPacket) LEGACY.skip("EntityInteract") LEGACY.skip("EntityInteractResult") LEGACY.skip("HitRequest") diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityCreatePacket.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityCreatePacket.kt index b734fa79..ece54dc8 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityCreatePacket.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityCreatePacket.kt @@ -35,6 +35,7 @@ class EntityCreatePacket(val entityType: EntityType, val storeData: ByteArrayLis override fun play(connection: ServerConnection) { if (entityID !in connection.entityIDRange) { LOGGER.error("Player $connection tried to create entity $entityType with ID $entityID, but that's outside of allowed range ${connection.entityIDRange}!") + connection.disconnect("Creating entity with ID $entityID outside of allowed range ${connection.entityIDRange}") } else { val entity = when (entityType) { EntityType.PLAYER -> { diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityDestroyPacket.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityDestroyPacket.kt new file mode 100644 index 00000000..cbcea78f --- /dev/null +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityDestroyPacket.kt @@ -0,0 +1,43 @@ +package ru.dbotthepony.kstarbound.network.packets + +import it.unimi.dsi.fastutil.bytes.ByteArrayList +import org.apache.logging.log4j.LogManager +import ru.dbotthepony.kommons.io.readByteArray +import ru.dbotthepony.kommons.io.readSignedVarInt +import ru.dbotthepony.kommons.io.writeByteArray +import ru.dbotthepony.kommons.io.writeSignedVarInt +import ru.dbotthepony.kstarbound.client.ClientConnection +import ru.dbotthepony.kstarbound.network.IClientPacket +import ru.dbotthepony.kstarbound.network.IServerPacket +import ru.dbotthepony.kstarbound.server.ServerConnection +import java.io.DataInputStream +import java.io.DataOutputStream + +class EntityDestroyPacket(val entityID: Int, val finalNetState: ByteArrayList, val isDeath: Boolean) : IClientPacket, IServerPacket { + constructor(stream: DataInputStream, isLegacy: Boolean) : this(stream.readSignedVarInt(), ByteArrayList.wrap(stream.readByteArray()), stream.readBoolean()) + + override fun write(stream: DataOutputStream, isLegacy: Boolean) { + stream.writeSignedVarInt(entityID) + stream.writeByteArray(finalNetState.elements(), 0, finalNetState.size) + stream.writeBoolean(isDeath) + } + + override fun play(connection: ServerConnection) { + if (entityID !in connection.entityIDRange) { + LOGGER.error("Client $connection tried to remove entity with ID $entityID, but that's outside of allowed range ${connection.entityIDRange}!") + connection.disconnect("Removing entity with ID $entityID outside of allowed range ${connection.entityIDRange}") + } else { + connection.enqueue { + entities[entityID]?.remove() + } + } + } + + override fun play(connection: ClientConnection) { + TODO("Not yet implemented") + } + + companion object { + private val LOGGER = LogManager.getLogger() + } +} \ No newline at end of file diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityUpdateSetPacket.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityUpdateSetPacket.kt index 97852435..83c13e3a 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityUpdateSetPacket.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/network/packets/EntityUpdateSetPacket.kt @@ -34,6 +34,8 @@ class EntityUpdateSetPacket(val forConnection: Int, val deltas: Int2ObjectMap { diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionSpinner.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionSpinner.kt index 0ad4e627..fbe4167d 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionSpinner.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/util/ExecutionSpinner.kt @@ -68,6 +68,7 @@ class ExecutionSpinner(private val waiter: Runnable, private val spinner: Boolea carrier = Thread.currentThread() while (isPaused) { + waiter.run() LockSupport.park() } @@ -77,10 +78,14 @@ class ExecutionSpinner(private val waiter: Runnable, private val spinner: Boolea waiter.run() diff = timeUntilNextFrame() - if (diff >= SYSTEM_SCHEDULER_RESOLUTION * 2L) - LockSupport.parkNanos(diff - SYSTEM_SCHEDULER_RESOLUTION) - else if (diff > SYSTEM_SCHEDULER_RESOLUTION) - LockSupport.parkNanos(SYSTEM_SCHEDULER_RESOLUTION) + if (PRECISE_WAIT) { + if (diff >= SYSTEM_SCHEDULER_RESOLUTION * 2L) + LockSupport.parkNanos(diff - SYSTEM_SCHEDULER_RESOLUTION) + else if (diff > SYSTEM_SCHEDULER_RESOLUTION) + LockSupport.parkNanos(SYSTEM_SCHEDULER_RESOLUTION) + } else { + LockSupport.parkNanos(diff) + } } val mark = System.nanoTime() @@ -98,6 +103,7 @@ class ExecutionSpinner(private val waiter: Runnable, private val spinner: Boolea } companion object { + private const val PRECISE_WAIT = false private val LOGGER = LogManager.getLogger() private var SYSTEM_SCHEDULER_RESOLUTION = 1_000_000L diff --git a/src/main/kotlin/ru/dbotthepony/kstarbound/world/entities/AbstractEntity.kt b/src/main/kotlin/ru/dbotthepony/kstarbound/world/entities/AbstractEntity.kt index b4392d34..c0ae14c5 100644 --- a/src/main/kotlin/ru/dbotthepony/kstarbound/world/entities/AbstractEntity.kt +++ b/src/main/kotlin/ru/dbotthepony/kstarbound/world/entities/AbstractEntity.kt @@ -8,6 +8,7 @@ import ru.dbotthepony.kstarbound.client.StarboundClient import ru.dbotthepony.kstarbound.client.render.LayeredRenderer import ru.dbotthepony.kstarbound.defs.EntityType import ru.dbotthepony.kstarbound.defs.JsonDriven +import ru.dbotthepony.kstarbound.network.packets.EntityDestroyPacket import ru.dbotthepony.kstarbound.network.syncher.MasterElement import ru.dbotthepony.kstarbound.network.syncher.NetworkedGroup import ru.dbotthepony.kstarbound.world.Chunk @@ -131,6 +132,7 @@ abstract class AbstractEntity(path: String) : JsonDriven(path) { check(world.entities.remove(entityID) == this) { "Tried to remove $this from $world, but removed something else!" } world.orphanedEntities.remove(this) onRemove(world) + world.broadcast(EntityDestroyPacket(entityID, ByteArrayList(), false)) innerWorld = null }