diff --git a/archive/src/main/kotlin/org/openrs2/archive/cache/CacheImporter.kt b/archive/src/main/kotlin/org/openrs2/archive/cache/CacheImporter.kt index 59a314fc..7d3e03c5 100644 --- a/archive/src/main/kotlin/org/openrs2/archive/cache/CacheImporter.kt +++ b/archive/src/main/kotlin/org/openrs2/archive/cache/CacheImporter.kt @@ -537,13 +537,7 @@ public class CacheImporter @Inject constructor( } } - // TODO(gpe): avoid uncompressing twice (we do it in isEncrypted and uncompress) - val uncompressed = if (Js5Compression.isEncrypted(buf.slice())) { - null - } else { - Js5Compression.uncompress(buf.slice()) - } - + val uncompressed = Js5Compression.uncompressUnlessEncrypted(buf.slice()) return Group(archive, group, buf.retain(), uncompressed, version, versionTruncated) } } catch (ex: IOException) { diff --git a/archive/src/main/kotlin/org/openrs2/archive/cache/Js5ChannelHandler.kt b/archive/src/main/kotlin/org/openrs2/archive/cache/Js5ChannelHandler.kt index 94a215c3..d0b547d9 100644 --- a/archive/src/main/kotlin/org/openrs2/archive/cache/Js5ChannelHandler.kt +++ b/archive/src/main/kotlin/org/openrs2/archive/cache/Js5ChannelHandler.kt @@ -133,13 +133,7 @@ public class Js5ChannelHandler( throw Exception("Group checksum invalid") } - // TODO(gpe): avoid uncompressing twice (we do it in isEncrypted and uncompress) - val uncompressed = if (Js5Compression.isEncrypted(response.data.slice())) { - null - } else { - Js5Compression.uncompress(response.data.slice()) - } - + val uncompressed = Js5Compression.uncompressUnlessEncrypted(response.data.slice()) groups += CacheImporter.Group( response.archive, response.group, diff --git a/archive/src/main/kotlin/org/openrs2/archive/key/KeyBruteForcer.kt b/archive/src/main/kotlin/org/openrs2/archive/key/KeyBruteForcer.kt index e2475843..47e27dfd 100644 --- a/archive/src/main/kotlin/org/openrs2/archive/key/KeyBruteForcer.kt +++ b/archive/src/main/kotlin/org/openrs2/archive/key/KeyBruteForcer.kt @@ -259,40 +259,29 @@ public class KeyBruteForcer @Inject constructor( keyId: Long, containerId: Long ): Boolean { - val valid = Unpooled.wrappedBuffer(data).use { buf -> - Js5Compression.isKeyValid(buf, key) - } - - if (!valid) { - return false - } + Unpooled.wrappedBuffer(data).use { buf -> + Js5Compression.uncompressIfKeyValid(buf, key).use { uncompressed -> + if (uncompressed == null) { + return false + } - // TODO(gpe): avoid uncompressing twice (we do it here and in isKeyValid) - var len = 0 - var crc32 = 0 + connection.prepareStatement( + """ + UPDATE containers + SET key_id = ?, uncompressed_length = ?, uncompressed_crc32 = ? + WHERE id = ? + """.trimIndent() + ).use { stmt -> + stmt.setLong(1, keyId) + stmt.setInt(2, uncompressed.readableBytes()) + stmt.setInt(3, uncompressed.crc32()) + stmt.setLong(4, containerId) + stmt.execute() + } - Unpooled.wrappedBuffer(data).use { buf -> - Js5Compression.uncompress(buf, key).use { uncompressed -> - len = uncompressed.readableBytes() - crc32 = uncompressed.crc32() + return true } } - - connection.prepareStatement( - """ - UPDATE containers - SET key_id = ?, uncompressed_length = ?, uncompressed_crc32 = ? - WHERE id = ? - """.trimIndent() - ).use { stmt -> - stmt.setLong(1, keyId) - stmt.setInt(2, len) - stmt.setInt(3, crc32) - stmt.setLong(4, containerId) - stmt.execute() - } - - return true } private companion object { diff --git a/cache/src/main/kotlin/org/openrs2/cache/Js5Compression.kt b/cache/src/main/kotlin/org/openrs2/cache/Js5Compression.kt index ff09f51d..3331840e 100644 --- a/cache/src/main/kotlin/org/openrs2/cache/Js5Compression.kt +++ b/cache/src/main/kotlin/org/openrs2/cache/Js5Compression.kt @@ -8,7 +8,6 @@ import org.openrs2.crypto.XteaKey import org.openrs2.crypto.xteaDecrypt import org.openrs2.crypto.xteaEncrypt import java.io.IOException -import java.io.OutputStream public object Js5Compression { private val BZIP2_MAGIC = byteArrayOf(0x31, 0x41, 0x59, 0x26, 0x53, 0x59) @@ -163,11 +162,11 @@ public object Js5Compression { } } - public fun isEncrypted(input: ByteBuf): Boolean { - return !isKeyValid(input, XteaKey.ZERO) + public fun uncompressUnlessEncrypted(input: ByteBuf): ByteBuf? { + return uncompressIfKeyValid(input, XteaKey.ZERO) } - public fun isKeyValid(input: ByteBuf, key: XteaKey): Boolean { + public fun uncompressIfKeyValid(input: ByteBuf, key: XteaKey): ByteBuf? { val typeId = input.readUnsignedByte().toInt() val type = Js5CompressionType.fromOrdinal(typeId) ?: throw IOException("Invalid compression type: $typeId") @@ -178,6 +177,10 @@ public object Js5Compression { } if (type == Js5CompressionType.UNCOMPRESSED) { + if (input.readableBytes() < len) { + throw IOException("Data truncated") + } + /* * There is no easy way for us to be sure whether an uncompressed * group's key is valid or not, as we'd need specific validation @@ -191,7 +194,11 @@ public object Js5Compression { * * We therefore assume all uncompressed groups are unencrypted. */ - return key.isZero + if (!key.isZero) { + return null + } + + return input.readBytes(len) } val lenWithUncompressedLen = len + 4 @@ -223,7 +230,7 @@ public object Js5Compression { decrypt(input.slice(), 16, key).use { plaintext -> val uncompressedLen = plaintext.readInt() if (uncompressedLen < 0) { - return false + return null } when (type) { @@ -232,19 +239,19 @@ public object Js5Compression { val magic = ByteArray(BZIP2_MAGIC.size) plaintext.readBytes(magic) if (!magic.contentEquals(BZIP2_MAGIC)) { - return false + return null } } Js5CompressionType.GZIP -> { val magic = plaintext.readUnsignedShort() if (magic != GZIP_MAGIC) { - return false + return null } // Jagex's implementation only supports DEFLATE. val compressionMethod = plaintext.readUnsignedByte().toInt() if (compressionMethod != GZIP_COMPRESSION_METHOD_DEFLATE) { - return false + return null } } Js5CompressionType.LZMA -> { @@ -257,7 +264,7 @@ public object Js5Compression { val pb = properties / 45 if (pb > LZMA_PB_MAX) { - return false + return null } /* @@ -279,9 +286,9 @@ public object Js5Compression { */ val dictSize = plaintext.readIntLE() if (dictSize < 0) { - return false + return null } else if (dictSize > LZMA_PRESET_DICT_SIZE_MAX) { - return false + return null } } } @@ -292,20 +299,43 @@ public object Js5Compression { val uncompressedLen = plaintext.readInt() check(uncompressedLen >= 0) - try { - OutputStream.nullOutputStream().use { output -> + /** + * We don't pass uncompressedLen to the buffer here: in some cases, + * an incorrect key can produce a valid header (particularly for + * LZMA, which has no magic number). If we're unlucky, + * uncompressedLen will be a huge number (e.g. 1 or 2 gigabytes), + * which might OOM some environments if allocated up front. + * + * However, if the key is incorrect it's likely that actually + * attempting to uncompress the data will quickly produce an error, + * long before we need to actually read 1 or 2 gigabytes of data. + * We therefore allow the buffer to grow dynamically. + */ + plaintext.alloc().buffer().use { output -> + try { type.createInputStream(ByteBufInputStream(plaintext, len), uncompressedLen).use { inputStream -> - if (inputStream.transferTo(output) != uncompressedLen.toLong()) { - return false + var remaining = uncompressedLen + while (remaining > 0) { + val n = output.writeBytes(inputStream, remaining) + if (n == -1) { + // uncompressed data truncated + return null + } + remaining -= n + } + + if (inputStream.read() != -1) { + // uncompressed data overflow + return null } } + } catch (ex: IOException) { + return null } - } catch (ex: IOException) { - return false + + return output.retain() } } - - return true } private fun decrypt(buf: ByteBuf, len: Int, key: XteaKey): ByteBuf { diff --git a/cache/src/test/kotlin/org/openrs2/cache/Js5CompressionTest.kt b/cache/src/test/kotlin/org/openrs2/cache/Js5CompressionTest.kt index 571d91a6..eec4cc54 100644 --- a/cache/src/test/kotlin/org/openrs2/cache/Js5CompressionTest.kt +++ b/cache/src/test/kotlin/org/openrs2/cache/Js5CompressionTest.kt @@ -9,9 +9,8 @@ import java.io.IOException import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFailsWith -import kotlin.test.assertFalse import kotlin.test.assertNotEquals -import kotlin.test.assertTrue +import kotlin.test.assertNull class Js5CompressionTest { @Test @@ -266,7 +265,7 @@ class Js5CompressionTest { } assertFailsWith { - Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO)?.release() } } } @@ -279,7 +278,7 @@ class Js5CompressionTest { } assertFailsWith { - Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO)?.release() } } } @@ -291,7 +290,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -311,7 +312,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -322,7 +325,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -333,7 +338,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -344,7 +351,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -355,7 +364,9 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @@ -366,92 +377,191 @@ class Js5CompressionTest { Js5Compression.uncompress(compressed.slice()).release() } - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { uncompressed -> + assertNull(uncompressed) + } } } @Test fun testNoneKeyValid() { - read("none.dat").use { compressed -> - assertFalse(Js5Compression.isEncrypted(compressed.slice())) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) + copiedBuffer("OpenRS2").use { expected -> + read("none.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } } } @Test fun testBzip2KeyValid() { - read("bzip2.dat").use { compressed -> - assertFalse(Js5Compression.isEncrypted(compressed.slice())) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) - } + copiedBuffer("OpenRS2").use { expected -> + read("bzip2.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertEquals(expected, actual) + } - read("bzip2-encrypted.dat").use { compressed -> - assertTrue(Js5Compression.isEncrypted(compressed.slice())) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } + + read("bzip2-encrypted.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } } read("bzip2-invalid-magic.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } } @Test fun testGzipKeyValid() { - read("gzip.dat").use { compressed -> - assertFalse(Js5Compression.isEncrypted(compressed.slice())) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) - } + copiedBuffer("OpenRS2").use { expected -> + read("gzip.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertEquals(expected, actual) + } - read("gzip-encrypted.dat").use { compressed -> - assertTrue(Js5Compression.isEncrypted(compressed.slice())) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } + + read("gzip-encrypted.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } } read("gzip-invalid-magic.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } read("gzip-invalid-method.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } } @Test fun testLzmaKeyValid() { - read("lzma.dat").use { compressed -> - assertFalse(Js5Compression.isEncrypted(compressed.slice())) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) - } + copiedBuffer("OpenRS2").use { expected -> + read("lzma.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } + + read("lzma-encrypted.dat").use { compressed -> + Js5Compression.uncompressUnlessEncrypted(compressed.slice()).use { actual -> + assertNull(actual) + } - read("lzma-encrypted.dat").use { compressed -> - assertTrue(Js5Compression.isEncrypted(compressed.slice())) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO)) - assertTrue(Js5Compression.isKeyValid(compressed.slice(), KEY)) - assertFalse(Js5Compression.isKeyValid(compressed.slice(), INVALID_KEY)) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO).use { actual -> + assertNull(actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), KEY).use { actual -> + assertEquals(expected, actual) + } + + Js5Compression.uncompressIfKeyValid(compressed.slice(), INVALID_KEY).use { actual -> + assertNull(actual) + } + } } read("lzma-dict-size-negative.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } read("lzma-dict-size-larger-than-preset.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } read("lzma-invalid-pb.dat").use { compressed -> - assertFalse(Js5Compression.isKeyValid(compressed, XteaKey.ZERO)) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO).use { actual -> + assertNull(actual) + } } } @@ -459,7 +569,7 @@ class Js5CompressionTest { fun testKeyValidShorterThanTwoBlocks() { read("shorter-than-two-blocks.dat").use { compressed -> assertFailsWith { - Js5Compression.isKeyValid(compressed, XteaKey.ZERO) + Js5Compression.uncompressIfKeyValid(compressed, XteaKey.ZERO)?.release() } } } @@ -472,7 +582,7 @@ class Js5CompressionTest { } assertFailsWith { - Js5Compression.isKeyValid(compressed.slice(), XteaKey.ZERO) + Js5Compression.uncompressIfKeyValid(compressed.slice(), XteaKey.ZERO)?.release() } } }