diff --git a/CHANGELOG.md b/CHANGELOG.md index a64da97e..62ff3843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ CHANGELOG ========= -4.1.1 +4.2.0 ------------------ * Fixed decoding of data pointers with offsets of 2 GiB or greater. The @@ -10,6 +10,19 @@ CHANGELOG with an `IllegalArgumentException`. Every record past the 2 GiB boundary was unreachable in databases larger than 2 GiB, which have been supported since 4.0.0. +* Fixed a denial-of-service issue in the decoder. A crafted database could nest + data-section pointers to shared targets so that decoding one record cost + exponential time and memory from a small file. The decoder now limits the + number of values it decodes for a single record and rejects a database that + exceeds it, along with pointer cycles and over-deep data, with an + `InvalidDatabaseException`. It also rejects a map or array whose declared size + is larger than the remaining data before allocating for it, so a crafted size + cannot force a large list or map allocation from a small file. A related shape + points many pointers at one large string or bytes value; the decoder now + bounds the total string and bytes payload it materializes for a single record, + charging each value as it is decoded, so re-decoding a shared target cannot + amplify a small file into gigabytes. This matches the reader resource limits + now recommended by the MaxMind DB specification. See GHSA-hj94-g986-h9r7. 4.1.0 (2026-05-12) ------------------ diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 73a337e5..e10b43d1 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -9,7 +9,6 @@ import java.lang.reflect.ParameterizedType; import java.math.BigInteger; import java.net.InetAddress; -import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; import java.nio.charset.CharsetDecoder; import java.nio.charset.StandardCharsets; @@ -35,6 +34,23 @@ class Decoder { private final NodeCache cache; + // Per-lookup decode limits recommended by the MaxMind DB specification. The + // depth limit stops pointer cycles and over-deep data before the stack + // overflows. The value limit stops a pointer fan-out, where nested pointers + // to shared targets would otherwise cost 2**depth decode operations. The + // payload limit stops a payload amplification, where many pointers to one + // large string or bytes value would otherwise materialize N times its size: + // each string or bytes value is charged its length every time it is decoded, + // so re-decoding a shared target recharges it. A Decoder serves a single + // lookup on a single thread, so these need no synchronization. The largest + // real records decode a few hundred values and a few kilobytes of payload. + private static final int MAX_DEPTH = 512; + private static final int MAX_VALUES = 1 << 16; + private static final long MAX_PAYLOAD_BYTES = 1 << 21; + private int depth; + private int valuesRemaining = MAX_VALUES; + private long payloadRemaining = MAX_PAYLOAD_BYTES; + private final long pointerBase; private final CharsetDecoder utfDecoder = UTF_8.newDecoder(); @@ -104,6 +120,9 @@ T decode(long offset, Class cls) throws IOException { + "pointer larger than the database."); } + this.valuesRemaining = MAX_VALUES; + this.payloadRemaining = MAX_PAYLOAD_BYTES; + this.depth = 0; this.buffer.position(offset); return cls.cast(decode(cls, null).value()); } @@ -122,6 +141,15 @@ private DecodedValue decode(CacheKey key) throws IOException { } private DecodedValue decode(Class cls, java.lang.reflect.Type genericType) + throws IOException { + if (--this.valuesRemaining < 0) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values"); + } + return decodeValue(cls, genericType); + } + + private DecodedValue decodeValue(Class cls, java.lang.reflect.Type genericType) throws IOException { var ctrlByte = 0xFF & this.buffer.get(); @@ -168,6 +196,16 @@ private DecodedValue decode(Class cls, java.lang.reflect.Type genericType DecodedValue decodePointer(long pointer, Class cls, java.lang.reflect.Type genericType) throws IOException { + // A pointer to another pointer is illegal per the specification. It also + // lets a pointer cycle recurse without ever entering a container, which + // the depth limit would not catch, so reject it here. Container cycles + // and over-deep data are bounded by the depth limit in decodeByType. + if (pointer < buffer.capacity() + && Type.fromControlByte(0xFF & buffer.get(pointer)) == Type.POINTER) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains a pointer to a pointer"); + } + var position = buffer.position(); var key = new CacheKey<>(pointer, cls, genericType); @@ -216,6 +254,45 @@ private static boolean isSimpleType(Class cls) { || cls.equals(BigInteger.class); } + // A container cannot hold more entries than there are bytes left to encode + // them: every key, value, and element occupies at least one byte. Reject an + // impossible declared size before it is used as an allocation hint, so a + // tiny crafted database cannot force a huge list or map preallocation and + // exhaust memory. valueCount is the number of encoded values the container + // declares (an array of N declares N, a map of N declares 2N). + private void checkContainerSize(long valueCount) throws InvalidDatabaseException { + // A container cannot decode more values than the per-lookup budget + // allows, so reject an oversized declaration before allocating for it + // rather than after the per-value limit stops the decode. + if (valueCount > this.valuesRemaining) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values"); + } + if (valueCount > this.buffer.capacity() - this.buffer.position()) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data: " + + "a container declares more entries than the data section can hold"); + } + } + + // Charge a string or bytes payload against the per-lookup budget before it + // is materialized. A payload amplification points many pointers at one large + // value; because the budget is charged every time the value is decoded, and + // a shared pointer target is re-decoded per referencing pointer, N pointers + // to an S-byte value are charged N*S and rejected once the total exceeds the + // limit. Charging before allocation also bounds an oversized variable-length + // integer, whose declared size the decoder would otherwise copy before + // range-checking. The comparison is against the remaining budget so it + // cannot overflow. The limit is inclusive: a total exactly at the limit is + // allowed. + private void chargePayload(long length) throws InvalidDatabaseException { + if (length > this.payloadRemaining) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum payload size"); + } + this.payloadRemaining -= length; + } + private Object decodeByType( Type type, int size, @@ -223,8 +300,16 @@ private Object decodeByType( java.lang.reflect.Type genericType ) throws IOException { switch (type) { - case MAP: - return this.decodeMap(size, cls, genericType); + case MAP: { + if (++this.depth > MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + this.checkContainerSize((long) size * 2); + var map = this.decodeMap(size, cls, genericType); + this.depth--; + return map; + } case ARRAY: Class elementClass = Object.class; if (genericType instanceof ParameterizedType ptype) { @@ -233,7 +318,14 @@ private Object decodeByType( elementClass = (Class) actualTypes[0]; } } - return this.decodeArray(size, cls, elementClass); + if (++this.depth > MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + this.checkContainerSize(size); + var array = this.decodeArray(size, cls, elementClass); + this.depth--; + return array; case BOOLEAN: Boolean bool = Decoder.decodeBoolean(size); return convertValue(bool, cls); @@ -377,7 +469,8 @@ private static Object coerceFromBigInteger(BigInteger value, Class target) { return value; } - private String decodeString(long size) throws CharacterCodingException { + private String decodeString(long size) throws IOException { + this.chargePayload(size); var oldLimit = buffer.limit(); buffer.limit(buffer.position() + size); var s = buffer.decode(utfDecoder); @@ -425,7 +518,7 @@ static int decodeInteger(Buffer buffer, int base, int size) { return integer; } - private BigInteger decodeBigInteger(int size) { + private BigInteger decodeBigInteger(int size) throws InvalidDatabaseException { var bytes = this.getByteArray(size); return new BigInteger(1, bytes); } @@ -1104,35 +1197,57 @@ private static Object parseDefault(String value, Class target) { private long nextValueOffset(long offset, int numberToSkip) throws InvalidDatabaseException { - if (numberToSkip == 0) { - return offset; - } - - var ctrlData = this.getCtrlData(offset); - var ctrlByte = ctrlData.ctrlByte(); - var size = ctrlData.size(); - offset = ctrlData.offset(); + // Iterate over siblings so a large flat unknown value cannot exhaust + // the Java stack. Recursion is only used to track structural nesting, + // which is bounded by the same limit as normal decoding. + for (var i = 0; i < numberToSkip; i++) { + if (--this.valuesRemaining < 0) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values"); + } - var type = ctrlData.type(); - switch (type) { - case POINTER: - var pointerSize = ((ctrlByte >>> 3) & 0x3) + 1; - offset += pointerSize; - break; - case MAP: - numberToSkip += 2 * size; - break; - case ARRAY: - numberToSkip += size; - break; - case BOOLEAN: - break; - default: - offset += size; - break; + var ctrlData = this.getCtrlData(offset); + var ctrlByte = ctrlData.ctrlByte(); + var size = ctrlData.size(); + offset = ctrlData.offset(); + + switch (ctrlData.type()) { + case POINTER: + var pointerSize = ((ctrlByte >>> 3) & 0x3) + 1; + offset += pointerSize; + break; + case MAP: + this.checkContainerSize((long) size * 2); + if (++this.depth > MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + try { + offset = this.nextValueOffset(offset, 2 * size); + } finally { + this.depth--; + } + break; + case ARRAY: + this.checkContainerSize(size); + if (++this.depth > MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + try { + offset = this.nextValueOffset(offset, size); + } finally { + this.depth--; + } + break; + case BOOLEAN: + break; + default: + offset += size; + break; + } } - - return nextValueOffset(offset, numberToSkip - 1); + return offset; } private CtrlData getCtrlData(long offset) @@ -1179,7 +1294,8 @@ private CtrlData getCtrlData(long offset) return new CtrlData(type, ctrlByte, offset, size); } - private byte[] getByteArray(int length) { + private byte[] getByteArray(int length) throws InvalidDatabaseException { + this.chargePayload(length); return Decoder.getByteArray(this.buffer, length); } diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index c68b1131..e44f8784 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -6,6 +6,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertThrows; +import java.io.ByteArrayOutputStream; import java.io.IOException; import java.math.BigInteger; import java.nio.charset.StandardCharsets; @@ -408,6 +409,225 @@ public void testInvalidControlByte() { containsString("The MaxMind DB file's data section contains bad data")); } + private static void writePointer1(ByteArrayOutputStream out, int target) { + // One-byte-payload pointer (type 1, pointer_size 1) with base 0. + out.write((1 << 5) | ((target >> 8) & 0x7)); + out.write(target & 0xFF); + } + + private static byte[] nestedArrays(int depth) { + var out = new ByteArrayOutputStream(); + for (var i = 0; i < depth; i++) { + out.write(0x01); // extended type, one element + out.write(0x04); // array + } + out.write(0xA0); // uint16 with value 0 + return out.toByteArray(); + } + + @Test + public void testPointerFanOutIsBounded() throws IOException { + // A data section of nested arrays, each holding two pointers to the + // node below, would cost 2**depth decode operations. The decoder bounds + // the number of values it decodes per lookup and rejects the database. + var depth = 100; + var out = new ByteArrayOutputStream(); + out.write(0xA0); // leaf: uint16 with value 0 + var prev = 0; + for (var i = 0; i < depth; i++) { + var offset = out.size(); + out.write(0x02); + out.write(0x04); + writePointer1(out, prev); + writePointer1(out, prev); + prev = offset; + } + + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(out.toByteArray()), 0); + var top = prev; + assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(top, Object.class)); + } + + @Test + public void testPointerFreeContainerDepthIsBounded() throws IOException { + var atLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(nestedArrays(512)), 0); + atLimit.decode(0, Object.class); + + var overLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(nestedArrays(513)), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> overLimit.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + + @Test + public void testUnknownFieldValueCountIsBounded() { + var out = new ByteArrayOutputStream(); + out.write(0xE1); // map with one key/value pair + out.write(0x47); // seven-byte UTF-8 string + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.write(0x1E); // extended type, two-byte size + out.write(0x04); // array + out.write(0xFE); // size = 65,535 + out.write(0xE2); + for (var i = 0; i < 65_535; i++) { + out.write(0xA0); // uint16 with value 0 + } + + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, EmptyModel.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + + @Test + public void testUnknownFieldDepthIsBounded() { + var value = nestedArrays(512); + var out = new ByteArrayOutputStream(); + out.write(0xE1); // map with one key/value pair + out.write(0x47); // seven-byte UTF-8 string + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.writeBytes(value); + + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, EmptyModel.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + + @Test + public void testHugeContainerIsRejectedBeforeAllocation() throws IOException { + // An array control byte can declare up to ~16.8 million entries from a + // few bytes. The value limit must reject this before the decoder uses + // the declared size as an allocation hint. + var out = new ByteArrayOutputStream(); + out.write(0x1F); // extended type, size code 31 (three size bytes) + out.write(0x04); // array + out.write(0xFF); // size = 65821 + 0xFFFFFF = 16,843,036 + out.write(0xFF); + out.write(0xFF); + + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(out.toByteArray()), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + + @Test + public void testImpossibleArrayIsRejectedBeforeAllocation() { + // The declared size is below the value budget, but two elements cannot + // be encoded in the one remaining byte. + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(new byte[] {0x02, 0x04, (byte) 0xA0}), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString( + "a container declares more entries than the data section can hold")); + } + + @Test + public void testImpossibleMapIsRejectedBeforeAllocation() { + // A one-entry map needs both a key and a value, but only one byte + // remains after its control byte. + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(new byte[] {(byte) 0xE1, (byte) 0xA0}), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString( + "a container declares more entries than the data section can hold")); + } + + @Test + public void testCyclicPointerThrows() { + // A pointer to itself must throw a catchable InvalidDatabaseException + // rather than recursing until the stack overflows. + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(new byte[] {0x20, 0x00}), 0); + assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + } + + @Test + public void testAcyclicPointerToPointerThrows() { + // The pointer chain terminates at a scalar, but pointer-to-pointer is + // illegal regardless of whether the chain forms a cycle. + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(new byte[] {0x20, 0x02, 0x20, 0x04, (byte) 0xA0}), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("pointer to a pointer")); + } + + // Writes a large scalar (bytes or string) at offset 0, followed by an array + // of pointerCount one-byte pointers that all target it. Every pointer + // re-decodes the shared value, so the decoder is charged its size once per + // pointer even though the value count stays tiny. + private static byte[] sharedScalarFanOut(int scalarType, int scalarSize, int pointerCount) { + var out = new ByteArrayOutputStream(); + // Scalar header: size code 30 covers 285..65820 bytes. + out.write((scalarType << 5) | 30); + var encoded = scalarSize - 285; + out.write((encoded >> 8) & 0xFF); + out.write(encoded & 0xFF); + for (var i = 0; i < scalarSize; i++) { + out.write(0); + } + // Array header (extended type 11), size code 29 covers 29..284 entries. + out.write(29); + out.write(0x04); + out.write(pointerCount - 29); + for (var i = 0; i < pointerCount; i++) { + writePointer1(out, 0); + } + return out.toByteArray(); + } + + @Test + public void testPayloadAmplificationIsBounded() throws IOException { + // 33 pointers to a 65,536-byte value would materialize just over 2 MiB, + // one byte value at a time, while the value count stays tiny. Only the + // payload byte bound rejects this. + var scalarSize = 1 << 16; + var data = sharedScalarFanOut(4, scalarSize, 33); + var top = 3 + scalarSize; + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(top, Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } + + @Test + public void testPayloadAtLimitIsAccepted() throws IOException { + // 32 pointers to a 65,536-byte value materialize exactly 2 MiB, at the + // inclusive limit, so the record must still decode. + var scalarSize = 1 << 16; + var data = sharedScalarFanOut(4, scalarSize, 32); + var top = 3 + scalarSize; + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + var result = (List) decoder.decode(top, Object.class); + assertEquals(32, result.size()); + } + + public static final class EmptyModel { + @MaxMindDbConstructor + public EmptyModel() { + } + } + private static void testTypeDecoding(Type type, Map tests) throws IOException { var cache = new CHMCache(); diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 9188677f..3bffae34 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2203,6 +2203,50 @@ public void testNullToPrimitiveErrorMessage(int chunkSize) throws IOException { } } + // A crafted database can point many data-section pointers at one large + // string or bytes value. The value count stays low, but a decoder that + // copies each pointer's target materializes N times its size. Decoding must + // reject each of these before it exhausts memory. + @Test + public void testPayloadAmplificationIsRejected() throws IOException { + var fixtures = new String[] { + "MaxMind-DB-test-payload-amplification-dos.mmdb", + "MaxMind-DB-test-payload-amplification-dos-string.mmdb", + "MaxMind-DB-test-payload-amplification-dos-worst-case.mmdb", + "MaxMind-DB-test-decoder-payload-limit-over.mmdb", + }; + var ip = InetAddress.getByName("1.1.1.1"); + for (var fixture : fixtures) { + try (var reader = new Reader(getFile(fixture))) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(ip, Object.class), + fixture + " should be rejected"); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } + } + } + + // A payload total that lands exactly on the 2 MiB limit is valid and must + // still decode, so the bound does not reject legitimate data. + @Test + public void testPayloadAtLimitDecodes() throws IOException { + try (var reader = new Reader(getFile("MaxMind-DB-test-decoder-payload-limit.mmdb"))) { + var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); + assertNotNull(value); + } + } + + // Metadata is decoded while the database is opened, so the payload bound must + // cover that path too. This fixture amplifies a string through the metadata. + @Test + public void testMetadataPayloadAmplificationIsRejected() { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> new Reader(getFile("MaxMind-DB-test-metadata-payload-limit.mmdb"))); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } + static File getFile(String name) { return new File(ReaderTest.class.getResource("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/maxmind-db/test-data/" + name).getFile()); } diff --git a/src/test/resources/maxmind-db b/src/test/resources/maxmind-db index b019327b..d692a4b7 160000 --- a/src/test/resources/maxmind-db +++ b/src/test/resources/maxmind-db @@ -1 +1 @@ -Subproject commit b019327b2c96a4efe08a9aa20c9e73150d104147 +Subproject commit d692a4b74c68c6e856d0bd85a38ee405b65c816f