From ba44543d4efa8b0970c50ad324a6bc7e643b1765 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 00:26:39 +0000 Subject: [PATCH 1/5] Bound decoder work to prevent a pointer fan-out denial of service A crafted data section could nest pointers to shared targets so that decoding one record cost exponential time and memory from a small file (GHSA-hj94-g986-h9r7). The decoder now limits the number of values it decodes for a single record and rejects a database that exceeds the limit with an InvalidDatabaseException. The limit is 65,536, far above the few hundred values the largest real records decode. To keep the guard cheap, the value count is checked per value while the depth limit is applied only when entering a map or array (the only places nesting deepens); a pointer to another pointer, which is illegal and lets a cycle recurse without entering a container, is rejected directly. Cycles and over-deep data are rejected the same way rather than exhausting the stack. This matches the reader resource limits now recommended by the MaxMind DB specification. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 9 +++- src/main/java/com/maxmind/db/Decoder.java | 51 +++++++++++++++++-- src/test/java/com/maxmind/db/DecoderTest.java | 43 ++++++++++++++++ 3 files changed, 99 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a64da97e..675e3093 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,13 @@ 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`. 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..74b14b28 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -35,6 +35,17 @@ 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. A + // Decoder serves a single lookup on a single thread, so these need no + // synchronization. The largest real records decode a few hundred values. + private static final int MAX_DEPTH = 512; + private static final int MAX_VALUES = 1 << 16; + private int depth; + private int valuesRemaining = MAX_VALUES; + private final long pointerBase; private final CharsetDecoder utfDecoder = UTF_8.newDecoder(); @@ -104,6 +115,8 @@ T decode(long offset, Class cls) throws IOException { + "pointer larger than the database."); } + this.valuesRemaining = MAX_VALUES; + this.depth = 0; this.buffer.position(offset); return cls.cast(decode(cls, null).value()); } @@ -122,6 +135,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 +190,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); @@ -223,8 +255,15 @@ 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"); + } + var map = this.decodeMap(size, cls, genericType); + this.depth--; + return map; + } case ARRAY: Class elementClass = Object.class; if (genericType instanceof ParameterizedType ptype) { @@ -233,7 +272,13 @@ 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"); + } + var array = this.decodeArray(size, cls, elementClass); + this.depth--; + return array; case BOOLEAN: Boolean bool = Decoder.decodeBoolean(size); return convertValue(bool, cls); diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index c68b1131..b549701e 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,48 @@ 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); + } + + @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 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)); + } + private static void testTypeDecoding(Type type, Map tests) throws IOException { var cache = new CHMCache(); From cf76d1853aa37f040f9270f330d5d34568d8b7f9 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 18:01:09 +0000 Subject: [PATCH 2/5] Reject oversized container sizes before allocating A map or array control byte can declare up to about 16.8 million entries from a few bytes. The decoder used the declared size as the initial capacity of a list or map before reading any element, so a crafted size forced a large allocation from a small file. A self-referential array with an oversized declared size compounded this, holding one such allocation per level until the depth limit stopped it, which could reach tens of gigabytes. The decoder now rejects a container whose declared size is larger than the bytes remaining in the data section, because every entry occupies at least one byte. This bounds the allocation to the size of the data section. The per-value limit added for the pointer fan-out does not catch this on its own, because the oversized allocation happens before any element is decoded. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 7 ++++-- src/main/java/com/maxmind/db/Decoder.java | 23 +++++++++++++++++++ src/test/java/com/maxmind/db/DecoderTest.java | 22 ++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 675e3093..7c29276f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,8 +15,11 @@ CHANGELOG 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`. This matches the reader resource limits now - recommended by the MaxMind DB specification. See GHSA-hj94-g986-h9r7. + `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. 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 74b14b28..2520fd4c 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -248,6 +248,27 @@ 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"); + } + } + private Object decodeByType( Type type, int size, @@ -260,6 +281,7 @@ private Object decodeByType( 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; @@ -276,6 +298,7 @@ private Object decodeByType( 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; diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index b549701e..bdb3e666 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -440,6 +440,28 @@ public void testPointerFanOutIsBounded() throws IOException { () -> decoder.decode(top, Object.class)); } + @Test + public void testOversizedContainerIsRejectedBeforeAllocation() throws IOException { + // An array control byte can declare up to ~16.8 million entries from a + // few bytes. Without a check, decoding preallocates a list that large + // before reading any element, and a self-referential oversized array + // recurses, holding one such allocation per level until the depth limit + // stops it: tens of gigabytes from a handful of bytes. The decoder must + // reject a declared size larger than the remaining data first. + 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); + writePointer1(out, 0); // element 0 points at the array itself + + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(out.toByteArray()), 0); + assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + } + @Test public void testCyclicPointerThrows() { // A pointer to itself must throw a catchable InvalidDatabaseException From 710ca80574f1d562a1a316a552467edaabc8be30 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 21:45:27 +0000 Subject: [PATCH 3/5] fixup! Bound decoder work to prevent a pointer fan-out denial of service --- src/main/java/com/maxmind/db/Decoder.java | 76 ++++++++++------- src/test/java/com/maxmind/db/DecoderTest.java | 81 +++++++++++++++++++ 2 files changed, 130 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 2520fd4c..d510aad0 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -1172,35 +1172,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) diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index bdb3e666..947009b8 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -415,6 +415,16 @@ private static void writePointer1(ByteArrayOutputStream out, int target) { 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 @@ -440,6 +450,59 @@ public void testPointerFanOutIsBounded() throws IOException { () -> 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 testOversizedContainerIsRejectedBeforeAllocation() throws IOException { // An array control byte can declare up to ~16.8 million entries from a @@ -473,6 +536,24 @@ public void testCyclicPointerThrows() { () -> 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")); + } + + public static final class EmptyModel { + @MaxMindDbConstructor + public EmptyModel() { + } + } + private static void testTypeDecoding(Type type, Map tests) throws IOException { var cache = new CHMCache(); From 872faeefb3206437bbc24332878e2f69c0e0ada7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 25 Aug 2026 21:45:58 +0000 Subject: [PATCH 4/5] fixup! Reject oversized container sizes before allocating --- src/test/java/com/maxmind/db/DecoderTest.java | 39 +++++++++++++++---- 1 file changed, 31 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 947009b8..b3307a1b 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -504,25 +504,48 @@ public void testUnknownFieldDepthIsBounded() { } @Test - public void testOversizedContainerIsRejectedBeforeAllocation() throws IOException { + public void testHugeContainerIsRejectedBeforeAllocation() throws IOException { // An array control byte can declare up to ~16.8 million entries from a - // few bytes. Without a check, decoding preallocates a list that large - // before reading any element, and a self-referential oversized array - // recurses, holding one such allocation per level until the depth limit - // stops it: tens of gigabytes from a handful of bytes. The decoder must - // reject a declared size larger than the remaining data first. + // 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); - writePointer1(out, 0); // element 0 points at the array itself var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(out.toByteArray()), 0); - assertThrows( + 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 From 5e85d2faa2db4ade3c8d4d0951db260676083ae9 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 27 Aug 2026 13:50:45 +0000 Subject: [PATCH 5/5] Bound the decoded string and bytes payload per lookup 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 target materializes the value once per pointer, so a file of a few hundred kilobytes can force gigabytes. The decoder now charges each string and bytes value its length as it is decoded and rejects a single lookup that materializes more than 2 MiB, with an InvalidDatabaseException. Because the charge is made every time a value is decoded, re-decoding a shared pointer target recharges it, so the amplification is bounded. Charging before allocation also bounds an oversized variable-length integer, whose declared size the decoder would otherwise copy before range-checking. Map keys and inline scalars in a pointed-to container decode through the same path, so they are charged too. Metadata is decoded through the same path, so the bound covers the database-open path as well. The counter is a per-lookup field on the per-lookup decoder, so concurrent reads stay thread-safe and the common path stays cheap: small fixed-width scalars are not charged. This matches the 2 MiB payload limit used by libmaxminddb and the Go reader. See GHSA-hj94-g986-h9r7. The test-data submodule is bumped to the MaxMind-DB commit that adds the payload amplification and boundary fixtures. Co-Authored-By: Claude Opus 4.8 --- CHANGELOG.md | 9 ++-- src/main/java/com/maxmind/db/Decoder.java | 40 ++++++++++++--- src/test/java/com/maxmind/db/DecoderTest.java | 51 +++++++++++++++++++ src/test/java/com/maxmind/db/ReaderTest.java | 44 ++++++++++++++++ src/test/resources/maxmind-db | 2 +- 5 files changed, 135 insertions(+), 11 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7c29276f..62ff3843 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,9 +17,12 @@ CHANGELOG 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. This matches - the reader resource limits now recommended by the MaxMind DB specification. - See GHSA-hj94-g986-h9r7. + 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 d510aad0..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; @@ -38,13 +37,19 @@ class Decoder { // 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. A - // Decoder serves a single lookup on a single thread, so these need no - // synchronization. The largest real records decode a few hundred values. + // 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; @@ -116,6 +121,7 @@ T decode(long offset, Class cls) throws IOException { } this.valuesRemaining = MAX_VALUES; + this.payloadRemaining = MAX_PAYLOAD_BYTES; this.depth = 0; this.buffer.position(offset); return cls.cast(decode(cls, null).value()); @@ -269,6 +275,24 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException } } + // 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, @@ -445,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); @@ -493,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); } @@ -1269,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 b3307a1b..e44f8784 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -571,6 +571,57 @@ public void testAcyclicPointerToPointerThrows() { 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() { 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