From 80e3bea0602ab7178b709bbe3569974d512685cf Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:32:32 +0000 Subject: [PATCH 01/25] Update decoder resource-limit test fixtures --- src/test/resources/maxmind-db | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/test/resources/maxmind-db b/src/test/resources/maxmind-db index b2a3df13..363086b7 160000 --- a/src/test/resources/maxmind-db +++ b/src/test/resources/maxmind-db @@ -1 +1 @@ -Subproject commit b2a3df13c0e274d7a2dca3d5415465a3a9670e23 +Subproject commit 363086b7d90650100e91f954937794c6a090c2a0 From 56300ff1bb450e458d2b149cdf8da3e270acbcb6 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:43:55 +0000 Subject: [PATCH 02/25] Bound decoder work per decode operation Reject decode operations that exceed 65,536 actual decode or skip operations or 128 nested containers. Apply the limits to metadata and unknown-field traversal, and reject pointer-to-pointer values before they can recurse. The Java-specific depth limit leaves stack headroom for pointer-backed maps on a 512 KiB thread stack. --- CHANGELOG.md | 5 +- src/main/java/com/maxmind/db/Decoder.java | 125 +++++-- src/test/java/com/maxmind/db/DecoderTest.java | 351 ++++++++++++++++++ src/test/java/com/maxmind/db/ReaderTest.java | 65 ++++ 4 files changed, 514 insertions(+), 32 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a64da97e..051df7f7 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,9 @@ 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. +* Added decoder limits to prevent excessive CPU and memory use from crafted + databases: 65,536 decoded or skipped values and 128 nested containers per + operation. Exceeding a limit throws `InvalidDatabaseException`. 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..892b9328 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -35,6 +35,21 @@ class Decoder { private final NodeCache cache; + // Per-operation resource limits. The MaxMind DB specification recommends + // depth and value limits, but permits equivalent reader-specific accounting. + // This decoder charges each decode invocation, including a pointer and its + // uncached target, so the value limit bounds actual decoder work rather than + // the specification's example flat value count. Container depth, together + // with rejecting illegal pointer-to-pointer values, bounds recursive calls. + // The lower depth limit leaves room on a 512 KiB thread stack even for + // pointer-backed maps, which use more Java frames per logical container + // than inline values. A Decoder serves one decode operation on one thread, + // so these fields need no synchronization. + private static final int MAX_DEPTH = 128; + 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 +119,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()); } @@ -115,7 +132,6 @@ private DecodedValue decode(CacheKey key) throws IOException { "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); } - this.buffer.position(offset); Class cls = key.cls(); return decode(cls, key.type()); @@ -123,6 +139,10 @@ 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"); + } var ctrlByte = 0xFF & this.buffer.get(); var type = Type.fromControlByte(ctrlByte); @@ -168,6 +188,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 +253,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 +270,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); @@ -1104,35 +1147,55 @@ 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: + 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: + 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 c68b1131..3b3b81a4 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -6,18 +6,23 @@ 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; +import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.TimeUnit; import org.junit.jupiter.api.Test; @SuppressWarnings({"boxing", "static-method"}) public class DecoderTest { + private static final int TEST_MAX_DEPTH = 128; + private static Map int32() { int max = (2 << 30) - 1; var int32 = new HashMap(); @@ -408,6 +413,352 @@ 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 void writePointer(ByteArrayOutputStream out, int target) { + if (target < 1 << 11) { + writePointer1(out, target); + return; + } + + var packed = target - (1 << 11); + out.write((1 << 5) | (1 << 3) | ((packed >> 16) & 0x7)); + out.write((packed >> 8) & 0xFF); + out.write(packed & 0xFF); + } + + // Array header for 29 or more elements. + private static void writeArrayHeader(ByteArrayOutputStream out, int size) { + if (size < 285) { + out.write(29); + out.write(0x04); + out.write(size - 29); + return; + } + + var encoded = size - 285; + out.write(30); + out.write(0x04); + out.write((encoded >> 8) & 0xFF); + out.write(encoded & 0xFF); + } + + // Map header for 285 or more key/value pairs. + private static void writeMapHeader(ByteArrayOutputStream out, int size) { + out.write(0xFE); + var encoded = size - 285; + out.write((encoded >>> 8) & 0xFF); + out.write(encoded & 0xFF); + } + + private static byte[] unknownFieldWithFlatArray(int size) { + 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)); + writeArrayHeader(out, size); + for (var i = 0; i < size; i++) { + out.write(0xA0); // uint16 with value 0 + } + return out.toByteArray(); + } + + private static byte[] unknownFieldWithFlatMap(int size) { + 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)); + writeMapHeader(out, size); + for (var i = 0; i < size; i++) { + out.write(0x40); // empty UTF-8 string key + out.write(0xA0); // uint16 with value 0 + } + return out.toByteArray(); + } + + 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(); + } + + private static byte[] nestedMaps(int depth) { + var out = new ByteArrayOutputStream(); + for (var i = 0; i < depth; i++) { + out.write(0xE1); // map with one key/value pair + out.write(0x40); // empty UTF-8 string key + } + out.write(0xA0); // uint16 with value 0 + return out.toByteArray(); + } + + private record EncodedValue(byte[] data, int offset) { + } + + private static EncodedValue pointerNestedArrays(int depth) { + var out = new ByteArrayOutputStream(); + out.write(0xA0); // uint16 with value 0 + var previous = 0; + for (var i = 0; i < depth; i++) { + var offset = out.size(); + out.write(0x01); // extended type, one element + out.write(0x04); // array + writePointer(out, previous); + previous = offset; + } + return new EncodedValue(out.toByteArray(), previous); + } + + private static EncodedValue pointerNestedMaps(int depth) { + var out = new ByteArrayOutputStream(); + out.write(0xA0); // uint16 with value 0 + var previous = 0; + for (var i = 0; i < depth; i++) { + var offset = out.size(); + out.write(0xE1); // map with one key/value pair + out.write(0x40); // empty UTF-8 string key + writePointer(out, previous); + previous = offset; + } + return new EncodedValue(out.toByteArray(), previous); + } + + private static byte[] inlineArray(int size) { + var out = new ByteArrayOutputStream(); + writeArrayHeader(out, size); + for (var i = 0; i < size; i++) { + 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(TEST_MAX_DEPTH)), 0); + atLimit.decode(0, Object.class); + + var overLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(nestedArrays(TEST_MAX_DEPTH + 1)), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> overLimit.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + + @Test + public void testPointerBackedContainerDepthIsBounded() throws IOException { + var atLimit = pointerNestedArrays(TEST_MAX_DEPTH); + var decoderAtLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(atLimit.data()), 0); + decoderAtLimit.decode(atLimit.offset(), Object.class); + + var overLimit = pointerNestedArrays(TEST_MAX_DEPTH + 1); + var decoderOverLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(overLimit.data()), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoderOverLimit.decode(overLimit.offset(), Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + + @Test + public void testContainerDepthFitsReducedThreadStack() throws Exception { + runProbe("-Xss512k", StackProbe.class); + } + + private static void runProbe(String vmArgument, Class probe) throws Exception { + var executable = System.getProperty("os.name").startsWith("Windows") + ? "java.exe" + : "java"; + var java = Path.of(System.getProperty("java.home"), "bin", executable).toString(); + var classPath = System.getProperty( + "surefire.test.class.path", + System.getProperty("java.class.path") + ); + var modulePath = System.getProperty("jdk.module.path"); + if (modulePath != null && !modulePath.isBlank()) { + classPath = String.join(System.getProperty("path.separator"), classPath, modulePath); + } + var process = new ProcessBuilder( + java, + vmArgument, + "-cp", + classPath, + probe.getName() + ).redirectErrorStream(true).start(); + + if (!process.waitFor(15, TimeUnit.SECONDS)) { + process.destroyForcibly(); + throw new AssertionError(probe.getSimpleName() + " did not finish within 15 seconds"); + } + var output = new String(process.getInputStream().readAllBytes(), StandardCharsets.UTF_8); + assertEquals(0, process.exitValue(), output); + } + + @Test + public void testJavaValueCountBoundary() throws IOException { + var atLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(inlineArray(65_535)), 0); + var result = (List) atLimit.decode(0, Object.class); + assertEquals(65_535, result.size()); + + var overLimit = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(inlineArray(65_536)), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> overLimit.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + + @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(TEST_MAX_DEPTH); + 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 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")); + } + + public static final class StackProbe { + private StackProbe() { + } + + public static void main(String[] args) throws IOException { + decode(nestedArrays(TEST_MAX_DEPTH), 0); + decode(nestedMaps(TEST_MAX_DEPTH), 0); + + var pointerArray = pointerNestedArrays(TEST_MAX_DEPTH); + decode(pointerArray.data(), pointerArray.offset()); + var pointerMap = pointerNestedMaps(TEST_MAX_DEPTH); + decode(pointerMap.data(), pointerMap.offset()); + + expectDepthRejection(nestedArrays(TEST_MAX_DEPTH + 1), 0); + expectDepthRejection(nestedMaps(TEST_MAX_DEPTH + 1), 0); + + pointerArray = pointerNestedArrays(TEST_MAX_DEPTH + 1); + expectDepthRejection(pointerArray.data(), pointerArray.offset()); + pointerMap = pointerNestedMaps(TEST_MAX_DEPTH + 1); + expectDepthRejection(pointerMap.data(), pointerMap.offset()); + + decodeUnknown(unknownFieldWithFlatArray(65_532)); + decodeUnknown(unknownFieldWithFlatMap(32_766)); + } + + private static void decode(byte[] data, int offset) throws IOException { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + decoder.decode(offset, Object.class); + } + + private static void expectDepthRejection(byte[] data, int offset) throws IOException { + try { + decode(data, offset); + throw new AssertionError("over-depth container decoded without rejection"); + } catch (InvalidDatabaseException e) { + if (!e.getMessage().contains("exceeds the maximum depth")) { + throw e; + } + } + } + + private static void decodeUnknown(byte[] data) throws IOException { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + decoder.decode(0, EmptyModel.class); + } + } + + 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..5ad2d79d 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2203,6 +2203,71 @@ public void testNullToPrimitiveErrorMessage(int chunkSize) throws IOException { } } + @ParameterizedTest + @MethodSource("chunkSizes") + public void testPointerFanOutIsRejected(int chunkSize) throws IOException { + var fixtures = new String[] { + "MaxMind-DB-test-pointer-decoder-dos.mmdb", + "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb", + }; + var addresses = new String[] {"1.1.1.1", "2001:db8::1"}; + for (var i = 0; i < fixtures.length; i++) { + var fixture = fixtures[i]; + try (var reader = new Reader(getFile(fixture), chunkSize)) { + var address = InetAddress.getByName(addresses[i]); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(address, Object.class), + fixture + " should be rejected"); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + } + } + + @Test + public void testPointerFanOutIsRejectedForMemoryAndStreamReaders() throws IOException { + var fixture = "MaxMind-DB-test-pointer-decoder-dos.mmdb"; + var address = InetAddress.getByName("1.1.1.1"); + try (var memoryReader = new Reader(getFile(fixture), FileMode.MEMORY, 512)) { + assertThrows( + InvalidDatabaseException.class, + () -> memoryReader.get(address, Object.class)); + } + try (var streamReader = new Reader(getStream(fixture), 512)) { + assertThrows( + InvalidDatabaseException.class, + () -> streamReader.get(address, Object.class)); + } + } + + @Test + public void testPointerFanOutUsesCachedTargets() throws IOException { + var fixture = "MaxMind-DB-test-pointer-decoder-dos.mmdb"; + try (var reader = new Reader(getFile(fixture), new CHMCache())) { + var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); + assertNotNull(value); + } + } + + @Test + public void testSharedValueFixturesUseJavaWorkAccounting() throws IOException { + var fixtures = new String[] { + "MaxMind-DB-test-decoder-value-limit.mmdb", + "MaxMind-DB-test-decoder-value-limit-over.mmdb", + "MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb", + }; + var address = InetAddress.getByName("1.1.1.1"); + for (var fixture : fixtures) { + try (var reader = new Reader(getFile(fixture))) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(address, Object.class), + fixture + " should be rejected under Java work accounting"); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + } + } + static File getFile(String name) { return new File(ReaderTest.class.getResource("/maxmind-db/test-data/" + name).getFile()); } From 7623b3c64c307ea4aac6116a5c3a7375a781560f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:44:47 +0000 Subject: [PATCH 03/25] Avoid revalidating cached pointer targets --- src/main/java/com/maxmind/db/Decoder.java | 18 ++++++++---------- src/test/java/com/maxmind/db/DecoderTest.java | 14 ++++++++------ 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 892b9328..374031b5 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -132,6 +132,14 @@ private DecodedValue decode(CacheKey key) throws IOException { "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); } + // Validate a target when the cache loader decodes it. A target that was + // loaded successfully has already passed this check, so cache hits do + // not need to reread its control byte. + if (Type.fromControlByte(0xFF & this.buffer.get(offset)) == Type.POINTER) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains a pointer to a pointer"); + } + this.buffer.position(offset); Class cls = key.cls(); return decode(cls, key.type()); @@ -188,16 +196,6 @@ 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); diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 3b3b81a4..94a4b870 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -698,12 +698,14 @@ public void testCyclicPointerThrows() { 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")); + var data = new byte[] {0x20, 0x02, 0x20, 0x04, (byte) 0xA0}; + for (var cache : List.of(NoCache.getInstance(), new CHMCache())) { + var decoder = new Decoder(cache, SingleBuffer.wrap(data), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class)); + assertThat(ex.getMessage(), containsString("pointer to a pointer")); + } } public static final class StackProbe { From 3eadcb5ad7ef98d9c1f8ea04361507da2e2c627b Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:45:16 +0000 Subject: [PATCH 04/25] Bound decoder collection preallocation --- src/main/java/com/maxmind/db/Decoder.java | 41 ++++- src/test/java/com/maxmind/db/DecoderTest.java | 149 ++++++++++++++++++ 2 files changed, 186 insertions(+), 4 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 374031b5..57201512 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -47,6 +47,12 @@ class Decoder { // so these fields need no synchronization. private static final int MAX_DEPTH = 128; private static final int MAX_VALUES = 1 << 16; + + // A collection's declared size is its logical child count, but it is not + // proof that the input contains that many decodable children. When deriving + // an initial capacity from it, limit unused capacity on the active recursion + // path; completed children remain bounded by MAX_VALUES. + private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; private int valuesRemaining = MAX_VALUES; @@ -244,6 +250,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, @@ -256,6 +283,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; @@ -272,6 +300,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; @@ -510,8 +539,9 @@ private List decodeArray( } List array; + var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); if (cls.equals(List.class) || cls.equals(Object.class)) { - array = new ArrayList<>(size); + array = new ArrayList<>(initialCapacity); } else { Constructor constructor; try { @@ -520,7 +550,7 @@ private List decodeArray( throw new DeserializationException( "No constructor found for the List: " + e.getMessage(), e); } - var parameters = new Object[]{size}; + var parameters = new Object[]{initialCapacity}; try { @SuppressWarnings("unchecked") var array2 = (List) constructor.newInstance(parameters); @@ -570,8 +600,9 @@ private Map decodeMapIntoMap( Class valueClass ) throws IOException { Map map; + var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); if (cls.equals(Map.class) || cls.equals(Object.class)) { - map = new HashMap<>(size); + map = new HashMap<>(initialCapacity); } else { Constructor constructor; try { @@ -580,7 +611,7 @@ private Map decodeMapIntoMap( throw new DeserializationException( "No constructor found for the Map: " + e.getMessage(), e); } - var parameters = new Object[]{size}; + var parameters = new Object[]{initialCapacity}; try { @SuppressWarnings("unchecked") var map2 = (Map) constructor.newInstance(parameters); @@ -1165,6 +1196,7 @@ private long nextValueOffset(long offset, int numberToSkip) 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"); @@ -1176,6 +1208,7 @@ private long nextValueOffset(long offset, int numberToSkip) } 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"); diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 94a4b870..9593bf2a 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -683,6 +683,84 @@ public void testUnknownFieldDepthIsBounded() { 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 testArrayInitialCapacityIsBounded() throws IOException { + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(inlineArray(129)), 0); + var result = decoder.decode(0, CapacityList.class); + assertEquals(128, result.initialCapacity); + assertEquals(129, result.size()); + } + + @Test + public void testMapInitialCapacityIsBounded() throws IOException { + var out = new ByteArrayOutputStream(); + out.write(0xFD); // map, size code 29 + out.write(100); // 29 + 100 = 129 entries + for (var i = 0; i < 129; i++) { + var key = Integer.toString(i).getBytes(StandardCharsets.UTF_8); + out.write(0x40 | key.length); + out.writeBytes(key); + out.write(0xA0); // uint16 with value 0 + } + + var decoder = new Decoder(NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), 0); + var result = decoder.decode(0, CapacityMap.class); + assertEquals(128, result.initialCapacity); + assertEquals(129, result.size()); + } + + @Test + public void testNestedLargeCollectionsDoNotExhaustHeap() throws Exception { + runProbe("-Xmx16m", AllocationProbe.class); + } + + @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 @@ -761,6 +839,77 @@ public EmptyModel() { } } + public static final class CapacityList extends ArrayList { + private static final long serialVersionUID = 1L; + private final int initialCapacity; + + public CapacityList(int initialCapacity) { + super(initialCapacity); + this.initialCapacity = initialCapacity; + } + } + + public static final class CapacityMap extends HashMap { + private static final long serialVersionUID = 1L; + private final int initialCapacity; + + public CapacityMap(int initialCapacity) { + super(initialCapacity); + this.initialCapacity = initialCapacity; + } + } + + public static final class AllocationProbe { + private AllocationProbe() { + } + + public static void main(String[] args) throws IOException { + decodeRecursivelyNestedArray(); + decodeRecursivelyNestedMap(); + } + + private static void decodeRecursivelyNestedArray() throws IOException { + var data = new byte[40_000]; + var encodedSize = 32_768 - 285; + data[0] = 0x1E; // extended type, size code 30 + data[1] = 0x04; // array + data[2] = (byte) (encodedSize >> 8); + data[3] = (byte) encodedSize; + data[4] = 0x20; // one-byte pointer to offset 0 + data[5] = 0x00; + + expectDepthRejection(data); + } + + private static void decodeRecursivelyNestedMap() throws IOException { + var data = new byte[40_000]; + var encodedSize = 16_384 - 285; + data[0] = (byte) 0xFE; // map, size code 30 + data[1] = (byte) (encodedSize >> 8); + data[2] = (byte) encodedSize; + data[3] = 0x41; // one-byte UTF-8 string key + data[4] = 'a'; + data[5] = (byte) 0xA0; // uint16 with value 0 + data[6] = 0x40; // empty UTF-8 string key + data[7] = 0x20; // one-byte pointer to offset 0 + data[8] = 0x00; + + expectDepthRejection(data); + } + + private static void expectDepthRejection(byte[] data) throws IOException { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + try { + decoder.decode(0, Object.class); + throw new AssertionError("nested large collection decoded without rejection"); + } catch (InvalidDatabaseException e) { + if (!e.getMessage().contains("exceeds the maximum depth")) { + throw e; + } + } + } + } + private static void testTypeDecoding(Type type, Map tests) throws IOException { var cache = new CHMCache(); From 3884fa5a1922bf6acb7080a625c50510664129fb Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:45:29 +0000 Subject: [PATCH 05/25] Avoid resizing small decoded maps --- src/main/java/com/maxmind/db/Decoder.java | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 57201512..d298263a 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -271,6 +271,16 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException } } + private static int initialMapCapacity(int size) { + // HashMap's constructor argument is a table capacity rather than an + // expected entry count. Account for its default 0.75 load factor when + // that can be done without exceeding the allocation-hint limit. + return Math.min( + size + (size + 2) / 3, + MAX_INITIAL_COLLECTION_CAPACITY + ); + } + private Object decodeByType( Type type, int size, @@ -600,10 +610,10 @@ private Map decodeMapIntoMap( Class valueClass ) throws IOException { Map map; - var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); if (cls.equals(Map.class) || cls.equals(Object.class)) { - map = new HashMap<>(initialCapacity); + map = new HashMap<>(initialMapCapacity(size)); } else { + var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); Constructor constructor; try { constructor = cls.getConstructor(Integer.TYPE); From 2eda46c44d0a06021facf08555f53f03e705b84c Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 18:38:16 +0000 Subject: [PATCH 06/25] Bound decoded payload per decode operation Reject an operation before it materializes more than 2 MiB of encoded string and bytes payload. Charge repeated cache misses while allowing cache hits to reuse completed target values. --- CHANGELOG.md | 6 +- src/main/java/com/maxmind/db/Decoder.java | 55 ++++++- src/test/java/com/maxmind/db/DecoderTest.java | 149 ++++++++++++++++++ src/test/java/com/maxmind/db/ReaderTest.java | 44 ++++++ 4 files changed, 244 insertions(+), 10 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 051df7f7..58ed1856 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,8 +11,10 @@ CHANGELOG boundary was unreachable in databases larger than 2 GiB, which have been supported since 4.0.0. * Added decoder limits to prevent excessive CPU and memory use from crafted - databases: 65,536 decoded or skipped values and 128 nested containers per - operation. Exceeding a limit throws `InvalidDatabaseException`. + databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB + of encoded string and bytes payload per operation. Exceeding a limit throws + `InvalidDatabaseException`. + * Truncated payloads and malformed UTF-8 are rejected. 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 d298263a..a6ee5b23 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -41,12 +41,15 @@ class Decoder { // uncached target, so the value limit bounds actual decoder work rather than // the specification's example flat value count. Container depth, together // with rejecting illegal pointer-to-pointer values, bounds recursive calls. + // The payload limit bounds encoded string and bytes data materialized by + // this Java decoder. // The lower depth limit leaves room on a 512 KiB thread stack even for // pointer-backed maps, which use more Java frames per logical container // than inline values. A Decoder serves one decode operation on one thread, // so these fields need no synchronization. private static final int MAX_DEPTH = 128; private static final int MAX_VALUES = 1 << 16; + private static final long MAX_PAYLOAD_BYTES = 1 << 21; // A collection's declared size is its logical child count, but it is not // proof that the input contains that many decodable children. When deriving @@ -55,6 +58,7 @@ class Decoder { private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; private int valuesRemaining = MAX_VALUES; + private long payloadRemaining = MAX_PAYLOAD_BYTES; private final long pointerBase; @@ -126,6 +130,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()); @@ -257,7 +262,7 @@ private static boolean isSimpleType(Class cls) { // 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 + // A container cannot decode more values than the per-operation 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) { @@ -271,6 +276,33 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException } } + // Charge a string or bytes payload against the per-operation 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.checkDataSize(length); + this.payloadRemaining -= length; + } + + private void checkDataSize(long length) throws InvalidDatabaseException { + if (length > this.buffer.capacity() - this.buffer.position()) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data: " + + "a value extends beyond the end of the data section."); + } + } + private static int initialMapCapacity(int size) { // HashMap's constructor argument is a table capacity rather than an // expected entry count. Account for its default 0.75 load factor when @@ -457,12 +489,18 @@ 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); - buffer.limit(oldLimit); - return s; + try { + buffer.limit(buffer.position() + size); + return buffer.decode(utfDecoder); + } catch (CharacterCodingException e) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains an invalid UTF-8 string", e); + } finally { + buffer.limit(oldLimit); + } } private int decodeUint16(int size) { @@ -505,7 +543,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); } @@ -1283,7 +1321,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 9593bf2a..4bd43a5a 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -786,6 +786,142 @@ public void testAcyclicPointerToPointerThrows() { } } + // 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 testPayloadAmplificationIsBoundedAfterCacheFills() { + var scalarSize = 1 << 16; + var data = sharedScalarFanOut(4, scalarSize, 33); + var top = 3 + scalarSize; + var decoder = new Decoder(new CHMCache(0), 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 testOverBudgetPayloadHeadersAreRejectedBeforePayloadRead() { + var overBudgetHeaders = List.of( + new byte[] {0x5F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF}, + new byte[] {(byte) 0x9F, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF} + ); + for (var header : overBudgetHeaders) { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(header), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class) + ); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } + } + + @Test + public void testTruncatedPayloadsAreRejectedAsInvalidDatabase() { + var headers = List.of( + new byte[] {0x41}, + new byte[] {(byte) 0x81} + ); + for (var header : headers) { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(header), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class) + ); + assertThat(ex.getMessage(), containsString("extends beyond the end")); + } + } + + @Test + public void testInvalidStringDoesNotChangeBufferLimit() throws IOException { + var data = new byte[] {0x41, (byte) 0xFF, 0x41, 'a'}; + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class) + ); + assertThat(ex.getMessage(), containsString("invalid UTF-8 string")); + assertEquals("a", decoder.decode(2, Object.class)); + } + + @Test + public void testSkippedPayloadDoesNotConsumeMaterializationBudget() throws IOException { + var payloadSize = (1 << 21) + 1; + var out = new ByteArrayOutputStream(); + out.write(0xE2); // map with two key/value pairs + out.write(0x47); // seven-byte UTF-8 string + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.write(0x5F); // UTF-8 string, size code 31 + var encodedSize = payloadSize - 65_821; + out.write((encodedSize >>> 16) & 0xFF); + out.write((encodedSize >>> 8) & 0xFF); + out.write(encodedSize & 0xFF); + out.writeBytes(new byte[payloadSize]); + out.write(0x45); // five-byte UTF-8 string + out.writeBytes("known".getBytes(StandardCharsets.UTF_8)); + out.write(0x42); // two-byte UTF-8 string + out.writeBytes("ok".getBytes(StandardCharsets.UTF_8)); + + var decoder = new Decoder( + NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), + 0 + ); + var result = decoder.decode(0, KnownFieldModel.class); + assertEquals("ok", result.known()); + } + + @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 StackProbe { private StackProbe() { } @@ -839,6 +975,19 @@ public EmptyModel() { } } + public static final class KnownFieldModel { + private final String known; + + @MaxMindDbConstructor + public KnownFieldModel(@MaxMindDbParameter(name = "known") String known) { + this.known = known; + } + + public String known() { + return this.known; + } + } + public static final class CapacityList extends ArrayList { private static final long serialVersionUID = 1L; private final int initialCapacity; diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 5ad2d79d..31bd870f 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2268,6 +2268,50 @@ public void testSharedValueFixturesUseJavaWorkAccounting() 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("/maxmind-db/test-data/" + name).getFile()); } From ee6d341ea6e31f325e3410b1a06954c2c6ee8ad7 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 20:23:24 +0000 Subject: [PATCH 07/25] Reject oversized integer encodings Validate every integer payload width before reading it. This prevents malformed fixed-width integers from turning repeated pointer targets into attacker-sized decode loops. --- CHANGELOG.md | 1 + src/main/java/com/maxmind/db/Decoder.java | 71 +++++++--- src/test/java/com/maxmind/db/DecoderTest.java | 126 ++++++++++++++++++ 3 files changed, 180 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58ed1856..2b392467 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,6 +14,7 @@ CHANGELOG databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws `InvalidDatabaseException`. + * Oversized integer encodings are rejected. * Truncated payloads and malformed UTF-8 are rejected. 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 a6ee5b23..d8cd98b9 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -277,15 +277,11 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException } // Charge a string or bytes payload against the per-operation 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. + // it is materialized. A payload amplification points many pointers at one large + // value. A cache miss that re-decodes the target charges its payload again; + // a cache hit reuses the completed value without materializing it again. 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( @@ -359,27 +355,46 @@ private Object decodeByType( case BYTES: return this.getByteArray(size); case UINT16: + this.checkIntegerSize("uint16", size, 2); return coerceFromInt(this.decodeUint16(size), cls); case UINT32: + this.checkIntegerSize("uint32", size, 4); return coerceFromLong(this.decodeUint32(size), cls); case INT32: + this.checkIntegerSize("int32", size, 4); return coerceFromInt(this.decodeInt32(size), cls); case UINT64: + this.checkIntegerSize("uint64", size, 8); + return this.decodeLargeUint(size, cls); case UINT128: - // Optimization: for typed fields, avoid BigInteger allocation when - // value fits in long. Keep Object.class behavior unchanged for - // backward compatibility. - if (size < 8 && !cls.equals(Object.class)) { - return coerceFromLong(this.decodeLong(size), cls); - } - // Size >= 8 bytes or Object.class target: use BigInteger - return coerceFromBigInteger(this.decodeBigInteger(size), cls); + this.checkIntegerSize("uint128", size, 16); + return this.decodeLargeUint(size, cls); default: throw new InvalidDatabaseException( "Unknown or unexpected type: " + type.name()); } } + private Object decodeLargeUint(int size, Class cls) + throws InvalidDatabaseException { + // For typed fields, avoid BigInteger allocation when the value fits in + // long. Keep Object.class behavior unchanged for backward compatibility. + if (size < 8 && !cls.equals(Object.class)) { + return coerceFromLong(this.decodeLong(size), cls); + } + return coerceFromBigInteger(this.decodeBigInteger(size), cls); + } + + private void checkIntegerSize(String type, int size, int maximum) + throws InvalidDatabaseException { + if (size > maximum) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data: " + + "invalid size of " + type + "."); + } + this.checkDataSize(size); + } + private static Object coerceFromInt(int value, Class target) { if (target.equals(Object.class) || target.equals(Integer.TYPE) @@ -544,7 +559,7 @@ static int decodeInteger(Buffer buffer, int base, int size) { } private BigInteger decodeBigInteger(int size) throws InvalidDatabaseException { - var bytes = this.getByteArray(size); + var bytes = Decoder.getByteArray(this.buffer, size); return new BigInteger(1, bytes); } @@ -1269,6 +1284,26 @@ private long nextValueOffset(long offset, int numberToSkip) break; case BOOLEAN: break; + case UINT16: + this.checkIntegerSize("uint16", size, 2); + offset += size; + break; + case UINT32: + this.checkIntegerSize("uint32", size, 4); + offset += size; + break; + case INT32: + this.checkIntegerSize("int32", size, 4); + offset += size; + break; + case UINT64: + this.checkIntegerSize("uint64", size, 8); + offset += size; + break; + case UINT128: + this.checkIntegerSize("uint128", size, 16); + offset += size; + break; default: offset += size; break; diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 4bd43a5a..c4560fa8 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -358,6 +358,106 @@ public void testUint128() throws IOException { DecoderTest.testTypeDecoding(Type.UINT128, largeUint(128)); } + @Test + public void testOversizedIntegersAreRejectedBeforePayloadRead() { + var invalidIntegers = Map.of( + "uint16", new byte[] {(byte) 0xA3}, + "uint32", new byte[] {(byte) 0xC5}, + "int32", new byte[] {0x05, 0x01}, + "uint64", new byte[] {0x09, 0x02}, + "uint128", new byte[] {0x11, 0x03} + ); + + for (var invalidInteger : invalidIntegers.entrySet()) { + var decoder = new Decoder( + NoCache.getInstance(), + SingleBuffer.wrap(invalidInteger.getValue()), + 0 + ); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class) + ); + assertThat(ex.getMessage(), containsString( + "invalid size of " + invalidInteger.getKey())); + } + } + + @Test + public void testTruncatedIntegersAreRejectedAsInvalidDatabase() { + var headers = List.of( + new byte[] {(byte) 0xA1}, + new byte[] {(byte) 0xC1}, + new byte[] {0x01, 0x01}, + new byte[] {0x01, 0x02}, + new byte[] {0x01, 0x03} + ); + + for (var header : headers) { + var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(header), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class) + ); + assertThat(ex.getMessage(), containsString("extends beyond the end")); + } + } + + @Test + public void testPointerBackedOversizedIntegerIsRejectedBeforePayloadRead() { + // A uint32 control byte can declare a 16,843,036-byte payload. The + // pointer target must be rejected before the decoder enters that loop. + var data = new byte[] { + (byte) 0xDF, (byte) 0xFF, (byte) 0xFF, (byte) 0xFF, + 0x20, 0x00 + }; + for (var cache : List.of(NoCache.getInstance(), new CHMCache())) { + var decoder = new Decoder(cache, SingleBuffer.wrap(data), 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(4, Object.class) + ); + assertThat(ex.getMessage(), containsString("invalid size of uint32")); + } + } + + @Test + public void testSkippedOversizedIntegersAreRejected() { + var invalidIntegers = Map.of( + "uint16", new byte[] {(byte) 0xA3, 0, 0, 0}, + "uint32", new byte[] {(byte) 0xC5, 0, 0, 0, 0, 0}, + "int32", new byte[] {0x05, 0x01, 0, 0, 0, 0, 0}, + "uint64", new byte[] {0x09, 0x02, 0, 0, 0, 0, 0, 0, 0, 0, 0}, + "uint128", new byte[] { + 0x11, 0x03, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0 + } + ); + + for (var invalidInteger : invalidIntegers.entrySet()) { + var out = new ByteArrayOutputStream(); + out.write(0xE2); // map with two key/value pairs + out.write(0x47); // seven-byte UTF-8 string + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.writeBytes(invalidInteger.getValue()); + out.write(0x45); // five-byte UTF-8 string + out.writeBytes("known".getBytes(StandardCharsets.UTF_8)); + out.write(0x42); // two-byte UTF-8 string + out.writeBytes("ok".getBytes(StandardCharsets.UTF_8)); + + var decoder = new Decoder( + NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), + 0 + ); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, KnownFieldModel.class) + ); + assertThat(ex.getMessage(), containsString( + "invalid size of " + invalidInteger.getKey())); + } + } + @Test public void testDoubles() throws IOException { DecoderTest @@ -922,6 +1022,32 @@ public void testPayloadAtLimitIsAccepted() throws IOException { assertEquals(32, result.size()); } + @Test + public void testBigIntegerDoesNotConsumeStringAndBytesBudget() throws IOException { + var payloadSize = 1 << 21; + var out = new ByteArrayOutputStream(); + out.write(0x02); // extended type, two elements + out.write(0x04); // array + out.write(0x10); // 16-byte extended value + out.write(0x03); // uint128 + out.writeBytes(new byte[16]); + + out.write(0x9F); // bytes, size code 31 + var encodedSize = payloadSize - 65_821; + out.write((encodedSize >>> 16) & 0xFF); + out.write((encodedSize >>> 8) & 0xFF); + out.write(encodedSize & 0xFF); + out.writeBytes(new byte[payloadSize]); + + var decoder = new Decoder( + NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), + 0 + ); + var result = (List) decoder.decode(0, Object.class); + assertEquals(2, result.size()); + } + public static final class StackProbe { private StackProbe() { } From d78374f49a5d34fbe5aa7176f9fcc66ac5d9737f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 20:24:39 +0000 Subject: [PATCH 08/25] Skip unknown pointer values by pointer width Do not parse pointer control bits as a generic payload size when skipping an unmapped typed field. Keep decoding aligned for all pointer widths and report truncated pointer payloads as invalid database data. --- CHANGELOG.md | 3 + src/main/java/com/maxmind/db/Decoder.java | 11 +++ src/test/java/com/maxmind/db/DecoderTest.java | 76 +++++++++++++++++++ 3 files changed, 90 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b392467..1b387845 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,9 @@ 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 skipping unknown four-byte pointers during typed decoding. Skipped + values that extend past the data section are now rejected. MaxMind-produced + databases were unaffected. * Added decoder limits to prevent excessive CPU and memory use from crafted databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index d8cd98b9..f9b0885f 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -1308,6 +1308,11 @@ private long nextValueOffset(long offset, int numberToSkip) offset += size; break; } + if (offset > this.buffer.capacity()) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data: " + + "a value extends beyond the end of the data section."); + } } return offset; } @@ -1342,6 +1347,12 @@ private CtrlData getCtrlData(long offset) offset++; } + // Pointer control bits encode pointer width and value bits, not a + // generic payload size. The caller advances by the pointer width. + if (type.equals(Type.POINTER)) { + return new CtrlData(type, ctrlByte, offset, 0); + } + var size = ctrlByte & 0x1f; if (size >= 29) { var bytesToRead = size - 28; diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index c4560fa8..815952cc 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -783,6 +783,82 @@ public void testUnknownFieldDepthIsBounded() { assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); } + @Test + public void testUnknownFieldPointersAreSkippedByTheirEncodedWidth() throws IOException { + var pointerEncodings = new ArrayList(); + pointerEncodings.add(new byte[] {0x20, 0x00}); + pointerEncodings.add(new byte[] {0x28, 0x00, 0x00}); + pointerEncodings.add(new byte[] {0x30, 0x00, 0x00, 0x00}); + for (var controlByte = 0x38; controlByte <= 0x3F; controlByte++) { + pointerEncodings.add(new byte[] { + (byte) controlByte, 0x00, 0x00, 0x00, 0x00 + }); + } + + for (var pointer : pointerEncodings) { + var out = new ByteArrayOutputStream(); + out.write(0xE2); // map with two key/value pairs + out.write(0x47); // seven-byte UTF-8 string + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.writeBytes(pointer); + out.write(0x45); // five-byte UTF-8 string + out.writeBytes("known".getBytes(StandardCharsets.UTF_8)); + out.write(0x42); // two-byte UTF-8 string + out.writeBytes("ok".getBytes(StandardCharsets.UTF_8)); + + var decoder = new Decoder( + NoCache.getInstance(), + SingleBuffer.wrap(out.toByteArray()), + 0 + ); + var result = decoder.decode(0, KnownFieldModel.class); + assertEquals("ok", result.known()); + } + } + + @Test + public void testTruncatedUnknownPointersAreRejectedAsInvalidDatabase() { + for (var pointerSize = 1; pointerSize <= 4; pointerSize++) { + 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(0x20 | ((pointerSize - 1) << 3)); + out.writeBytes(new byte[pointerSize - 1]); + + 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("extends beyond the end")); + } + } + + @Test + public void testTruncatedUnknownScalarIsRejectedAsInvalidDatabase() { + 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(0x41); // one-byte UTF-8 string with no payload + + 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("extends beyond the end")); + } + @Test public void testHugeContainerIsRejectedBeforeAllocation() throws IOException { // An array control byte can declare up to ~16.8 million entries from a From d448fb1b05721537b29898cd18d03dff83bc73b5 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 21:59:43 +0000 Subject: [PATCH 09/25] Restore container depth after decode failures Centralize container-entry validation and restore the current depth in finally blocks for both decoded and skipped containers. This keeps a cache loader that handles an IOException from leaking depth into the rest of the operation. --- src/main/java/com/maxmind/db/Decoder.java | 45 +++++++++++------------ 1 file changed, 21 insertions(+), 24 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index f9b0885f..f67776ef 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -276,6 +276,15 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException } } + private void enterContainer(long valueCount) throws InvalidDatabaseException { + this.checkContainerSize(valueCount); + if (this.depth >= MAX_DEPTH) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + this.depth++; + } + // Charge a string or bytes payload against the per-operation budget before // it is materialized. A payload amplification points many pointers at one large // value. A cache miss that re-decodes the target charges its payload again; @@ -317,14 +326,12 @@ private Object decodeByType( ) throws IOException { switch (type) { case MAP: { - if (++this.depth > MAX_DEPTH) { - throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum depth"); + this.enterContainer((long) size * 2); + try { + return this.decodeMap(size, cls, genericType); + } finally { + this.depth--; } - this.checkContainerSize((long) size * 2); - var map = this.decodeMap(size, cls, genericType); - this.depth--; - return map; } case ARRAY: Class elementClass = Object.class; @@ -334,14 +341,12 @@ private Object decodeByType( elementClass = (Class) actualTypes[0]; } } - if (++this.depth > MAX_DEPTH) { - throw new InvalidDatabaseException( - "The MaxMind DB file's data section exceeds the maximum depth"); + this.enterContainer(size); + try { + return this.decodeArray(size, cls, elementClass); + } finally { + this.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); @@ -1259,11 +1264,7 @@ private long nextValueOffset(long offset, int numberToSkip) 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"); - } + this.enterContainer((long) size * 2); try { offset = this.nextValueOffset(offset, 2 * size); } finally { @@ -1271,11 +1272,7 @@ private long nextValueOffset(long offset, int numberToSkip) } 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"); - } + this.enterContainer(size); try { offset = this.nextValueOffset(offset, size); } finally { From 0f7474080f339c79e05b0163760925a2216ecb5a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 21:59:53 +0000 Subject: [PATCH 10/25] Charge cached pointer targets by logical cost Retain each cached target's value, payload, and nesting costs, and charge them for every pointer occurrence. This keeps resource limits independent of cache state while preserving direct decoding for NoCache and context-dependent models. --- .../java/com/maxmind/db/DecodedValue.java | 39 +++- src/main/java/com/maxmind/db/Decoder.java | 64 +++++- src/test/java/com/maxmind/db/DecoderTest.java | 213 ++++++++++-------- src/test/java/com/maxmind/db/ReaderTest.java | 80 ++++--- 4 files changed, 261 insertions(+), 135 deletions(-) diff --git a/src/main/java/com/maxmind/db/DecodedValue.java b/src/main/java/com/maxmind/db/DecodedValue.java index 5440a684..7101186d 100644 --- a/src/main/java/com/maxmind/db/DecodedValue.java +++ b/src/main/java/com/maxmind/db/DecodedValue.java @@ -4,13 +4,50 @@ * {@code DecodedValue} is a wrapper for the decoded value. */ public final class DecodedValue { - final Object value; + private static final int PAYLOAD_SHIFT = 8; + private static final int VALUES_SHIFT = 30; + private static final long PAYLOAD_MASK = (1L << 22) - 1; + + Object value; DecodedValue(Object value) { this.value = value; } Object value() { + if (value instanceof CostedValue costedValue) { + return costedValue.value(); + } return value; } + + int values() { + return (int) (costs() >>> VALUES_SHIFT); + } + + long payloadBytes() { + return (costs() >>> PAYLOAD_SHIFT) & PAYLOAD_MASK; + } + + int depth() { + return (int) (costs() & 0xFF); + } + + DecodedValue costs(int values, long payloadBytes, int depth) { + var costs = ((long) values << VALUES_SHIFT) + | (payloadBytes << PAYLOAD_SHIFT) + | depth; + this.value = new CostedValue(this.value, costs); + return this; + } + + private long costs() { + if (value instanceof CostedValue costedValue) { + return costedValue.costs(); + } + return 0; + } + + private record CostedValue(Object value, long costs) { + } } diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index f67776ef..f54497b2 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -37,10 +37,12 @@ class Decoder { // Per-operation resource limits. The MaxMind DB specification recommends // depth and value limits, but permits equivalent reader-specific accounting. - // This decoder charges each decode invocation, including a pointer and its - // uncached target, so the value limit bounds actual decoder work rather than - // the specification's example flat value count. Container depth, together - // with rejecting illegal pointer-to-pointer values, bounds recursive calls. + // This decoder charges each decoded or skipped value. Each pointer occurrence + // also consumes the logical cost of its target. Cache misses measure that cost, + // and cache hits replay it. This keeps accounting independent of cache state + // rather than following the specification's example flat value count. + // Container depth, together with rejecting illegal pointer-to-pointer values, + // bounds recursive calls. // The payload limit bounds encoded string and bytes data materialized by // this Java decoder. // The lower depth limit leaves room on a 512 KiB thread stack even for @@ -57,6 +59,7 @@ class Decoder { // path; completed children remain bounded by MAX_VALUES. private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; + private int maxDepth; private int valuesRemaining = MAX_VALUES; private long payloadRemaining = MAX_PAYLOAD_BYTES; @@ -120,7 +123,7 @@ class Decoder { this.lookupNetwork = lookupNetwork; } - private final NodeCache.Loader cacheLoader = this::decode; + private final NodeCache.Loader cacheLoader = this::decodeForCache; T decode(long offset, Class cls) throws IOException { if (offset >= this.buffer.capacity()) { @@ -132,6 +135,7 @@ T decode(long offset, Class cls) throws IOException { this.valuesRemaining = MAX_VALUES; this.payloadRemaining = MAX_PAYLOAD_BYTES; this.depth = 0; + this.maxDepth = 0; this.buffer.position(offset); return cls.cast(decode(cls, null).value()); } @@ -205,22 +209,63 @@ private DecodedValue decode(Class cls, java.lang.reflect.Type genericType return new DecodedValue(this.decodeByType(type, size, cls, genericType)); } + private DecodedValue decodeForCache(CacheKey key) throws IOException { + var valuesRemaining = this.valuesRemaining; + var payloadRemaining = this.payloadRemaining; + var depth = this.depth; + var maxDepth = this.maxDepth; + this.maxDepth = depth; + try { + var value = this.decode(key); + return value.costs( + valuesRemaining - this.valuesRemaining, + payloadRemaining - this.payloadRemaining, + this.maxDepth - depth + ); + } finally { + this.valuesRemaining = valuesRemaining; + this.payloadRemaining = payloadRemaining; + this.depth = depth; + this.maxDepth = maxDepth; + } + } + DecodedValue decodePointer(long pointer, Class cls, java.lang.reflect.Type genericType) throws IOException { var position = buffer.position(); var key = new CacheKey<>(pointer, cls, genericType); DecodedValue value; - if (requiresLookupContext(cls)) { + if (this.cache == NoCache.getInstance() || requiresLookupContext(cls)) { value = this.decode(key); } else { value = cache.get(key, cacheLoader); + this.charge(value); } buffer.position(position); return value; } + private void charge(DecodedValue value) throws InvalidDatabaseException { + if (value.values() > this.valuesRemaining) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum number of values"); + } + if (value.payloadBytes() > this.payloadRemaining) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum payload size"); + } + if (value.depth() > MAX_DEPTH - this.depth) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section exceeds the maximum depth"); + } + + this.valuesRemaining -= value.values(); + this.payloadRemaining -= value.payloadBytes(); + this.maxDepth = Math.max(this.maxDepth, this.depth + value.depth()); + } + private boolean requiresLookupContext(Class cls) { if (cls == null || cls.equals(Object.class) @@ -283,12 +328,13 @@ private void enterContainer(long valueCount) throws InvalidDatabaseException { "The MaxMind DB file's data section exceeds the maximum depth"); } this.depth++; + this.maxDepth = Math.max(this.maxDepth, this.depth); } // Charge a string or bytes payload against the per-operation budget before - // it is materialized. A payload amplification points many pointers at one large - // value. A cache miss that re-decodes the target charges its payload again; - // a cache hit reuses the completed value without materializing it again. The + // materializing it. A payload amplification points many pointers at one large + // value. Cached targets retain their logical payload cost, so each pointer + // occurrence consumes the cost even when the decoder reuses the value. 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 { diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 815952cc..0db22081 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -23,6 +23,14 @@ public class DecoderTest { private static final int TEST_MAX_DEPTH = 128; + @Test + public void testDecodedValueStoresMaximumCosts() { + var value = new DecodedValue(null).costs(1 << 16, 1L << 21, TEST_MAX_DEPTH); + assertEquals(1 << 16, value.values()); + assertEquals(1L << 21, value.payloadBytes()); + assertEquals(TEST_MAX_DEPTH, value.depth()); + } + private static Map int32() { int max = (2 << 30) - 1; var int32 = new HashMap(); @@ -513,15 +521,11 @@ 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 void writePointer(ByteArrayOutputStream out, int target) { if (target < 1 << 11) { - writePointer1(out, target); + // One-byte-payload pointer (type 1, pointer_size 1) with base 0. + out.write((1 << 5) | ((target >> 8) & 0x7)); + out.write(target & 0xFF); return; } @@ -640,31 +644,6 @@ private static byte[] inlineArray(int size) { 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(), @@ -695,6 +674,30 @@ public void testPointerBackedContainerDepthIsBounded() throws IOException { assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); } + @Test + public void testCachedPointerTargetDepthIsBounded() throws IOException { + var nested = pointerNestedArrays(TEST_MAX_DEPTH); + var out = new ByteArrayOutputStream(); + out.writeBytes(nested.data()); + + var seedPointerOffset = out.size(); + writePointer(out, nested.offset()); + + var outerArrayOffset = out.size(); + out.write(0x01); // extended type, one element + out.write(0x04); // array + writePointer(out, nested.offset()); + + var decoder = new Decoder(new CHMCache(), SingleBuffer.wrap(out.toByteArray()), 0); + decoder.decode(seedPointerOffset, Object.class); + + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(outerArrayOffset, Object.class) + ); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + @Test public void testContainerDepthFitsReducedThreadStack() throws Exception { runProbe("-Xss512k", StackProbe.class); @@ -744,6 +747,56 @@ public void testJavaValueCountBoundary() throws IOException { assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); } + @Test + public void testPointerValueCountBoundaryIsIndependentOfCacheState() throws IOException { + // A warm cache replays a recorded cost instead of decoding the target + // again. The verdict at the boundary must not depend on which of those + // paths ran, so decode each fixture twice against the same cache. + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + var atLimit = new Decoder(cache, SingleBuffer.wrap(pointerFanOut(1)), 0); + for (var attempt = 0; attempt < 2; attempt++) { + var result = (List) atLimit.decode(1, Object.class); + assertEquals(32_768, result.size()); + } + } + + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + var overLimit = new Decoder(cache, SingleBuffer.wrap(pointerFanOut(2)), 0); + for (var attempt = 0; attempt < 2; attempt++) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> overLimit.decode(1, Object.class)); + assertThat(ex.getMessage(), + containsString("exceeds the maximum number of values")); + } + } + } + + // Builds an array of 32,767 pointers to one uint16, followed by the given + // number of inline uint16 values. Each pointer occurrence costs one value + // for the pointer and one for its target, so the decode costs + // 65,535 + scalars values. One scalar lands on the 65,536 limit and two + // exceed it by one. + private static byte[] pointerFanOut(int scalars) { + var pointers = 32_767; + var elements = pointers + scalars; + var out = new ByteArrayOutputStream(); + out.write(0xA0); // uint16 with value 0, the shared pointer target + out.write(0x1E); // extended type, size code 30 + out.write(0x04); // array + out.write((elements - 285) >> 8); + out.write(elements - 285); + for (var i = 0; i < pointers; i++) { + writePointer(out, 0); + } + for (var i = 0; i < scalars; i++) { + out.write(0xA0); + } + return out.toByteArray(); + } + @Test public void testUnknownFieldValueCountIsBounded() { var out = new ByteArrayOutputStream(); @@ -962,58 +1015,6 @@ public void testAcyclicPointerToPointerThrows() { } } - // 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 testPayloadAmplificationIsBoundedAfterCacheFills() { - var scalarSize = 1 << 16; - var data = sharedScalarFanOut(4, scalarSize, 33); - var top = 3 + scalarSize; - var decoder = new Decoder(new CHMCache(0), 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 testOverBudgetPayloadHeadersAreRejectedBeforePayloadRead() { var overBudgetHeaders = List.of( @@ -1086,18 +1087,6 @@ public void testSkippedPayloadDoesNotConsumeMaterializationBudget() throws IOExc assertEquals("ok", result.known()); } - @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()); - } - @Test public void testBigIntegerDoesNotConsumeStringAndBytesBudget() throws IOException { var payloadSize = 1 << 21; @@ -1134,29 +1123,53 @@ public static void main(String[] args) throws IOException { var pointerArray = pointerNestedArrays(TEST_MAX_DEPTH); decode(pointerArray.data(), pointerArray.offset()); + for (var cache : caches()) { + decode(pointerArray.data(), pointerArray.offset(), cache); + } var pointerMap = pointerNestedMaps(TEST_MAX_DEPTH); decode(pointerMap.data(), pointerMap.offset()); + for (var cache : caches()) { + decode(pointerMap.data(), pointerMap.offset(), cache); + } expectDepthRejection(nestedArrays(TEST_MAX_DEPTH + 1), 0); expectDepthRejection(nestedMaps(TEST_MAX_DEPTH + 1), 0); pointerArray = pointerNestedArrays(TEST_MAX_DEPTH + 1); expectDepthRejection(pointerArray.data(), pointerArray.offset()); + for (var cache : caches()) { + expectDepthRejection(pointerArray.data(), pointerArray.offset(), cache); + } pointerMap = pointerNestedMaps(TEST_MAX_DEPTH + 1); expectDepthRejection(pointerMap.data(), pointerMap.offset()); + for (var cache : caches()) { + expectDepthRejection(pointerMap.data(), pointerMap.offset(), cache); + } decodeUnknown(unknownFieldWithFlatArray(65_532)); decodeUnknown(unknownFieldWithFlatMap(32_766)); } private static void decode(byte[] data, int offset) throws IOException { - var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); + decode(data, offset, NoCache.getInstance()); + } + + private static void decode(byte[] data, int offset, NodeCache cache) throws IOException { + var decoder = new Decoder(cache, SingleBuffer.wrap(data), 0); decoder.decode(offset, Object.class); } private static void expectDepthRejection(byte[] data, int offset) throws IOException { + expectDepthRejection(data, offset, NoCache.getInstance()); + } + + private static void expectDepthRejection( + byte[] data, + int offset, + NodeCache cache + ) throws IOException { try { - decode(data, offset); + decode(data, offset, cache); throw new AssertionError("over-depth container decoded without rejection"); } catch (InvalidDatabaseException e) { if (!e.getMessage().contains("exceeds the maximum depth")) { @@ -1165,6 +1178,10 @@ private static void expectDepthRejection(byte[] data, int offset) throws IOExcep } } + private static List caches() { + return List.of(new CHMCache(), new CHMCache(0)); + } + private static void decodeUnknown(byte[] data) throws IOException { var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); decoder.decode(0, EmptyModel.class); diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 31bd870f..c998d28c 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2241,29 +2241,41 @@ public void testPointerFanOutIsRejectedForMemoryAndStreamReaders() throws IOExce } @Test - public void testPointerFanOutUsesCachedTargets() throws IOException { + public void testPointerFanOutIsRejectedWithCachedTargets() throws IOException { var fixture = "MaxMind-DB-test-pointer-decoder-dos.mmdb"; try (var reader = new Reader(getFile(fixture), new CHMCache())) { - var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); - assertNotNull(value); + var address = InetAddress.getByName("1.1.1.1"); + for (var i = 0; i < 2; i++) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(address, Object.class) + ); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + } + } + + public static final class TargetModel { + final String target; + + @MaxMindDbConstructor + public TargetModel(@MaxMindDbParameter(name = "target") String target) { + this.target = target; } } @Test - public void testSharedValueFixturesUseJavaWorkAccounting() throws IOException { - var fixtures = new String[] { - "MaxMind-DB-test-decoder-value-limit.mmdb", - "MaxMind-DB-test-decoder-value-limit-over.mmdb", - "MaxMind-DB-test-decoder-value-limit-pointer-heavy.mmdb", - }; + public void testPointerBackedMapKeysSharePayloadBudget() throws IOException { + var fixture = "MaxMind-DB-test-decode-path-shared-budget.mmdb"; var address = InetAddress.getByName("1.1.1.1"); - for (var fixture : fixtures) { - try (var reader = new Reader(getFile(fixture))) { + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + try (var reader = new Reader(getFile(fixture), cache)) { var ex = assertThrows( InvalidDatabaseException.class, - () -> reader.get(address, Object.class), - fixture + " should be rejected under Java work accounting"); - assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + () -> reader.get(address, TargetModel.class) + ); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); } } } @@ -2282,12 +2294,18 @@ public void testPayloadAmplificationIsRejected() throws IOException { }; 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")); + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + try (var reader = new Reader(getFile(fixture), cache)) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(ip, Object.class), + fixture + " should be rejected"); + assertThat( + ex.getMessage(), + containsString("exceeds the maximum payload size") + ); + } } } } @@ -2296,9 +2314,13 @@ public void testPayloadAmplificationIsRejected() throws IOException { // 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); + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + try (var reader = new Reader( + getFile("MaxMind-DB-test-decoder-payload-limit.mmdb"), cache)) { + var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); + assertNotNull(value); + } } } @@ -2306,10 +2328,14 @@ public void testPayloadAtLimitDecodes() throws IOException { // 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")); + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> new Reader(getFile("MaxMind-DB-test-metadata-payload-limit.mmdb"), cache) + ); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } } static File getFile(String name) { From e23b67b0fcc49cf2d0b86f5e82b8cbff381b5947 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 20:27:01 +0000 Subject: [PATCH 11/25] Document decoder resource limits --- CHANGELOG.md | 2 +- UPGRADING.md | 30 +++++++++++++++++++++++ src/main/java/com/maxmind/db/Decoder.java | 2 +- 3 files changed, 32 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1b387845..e39618c6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ CHANGELOG * Added decoder limits to prevent excessive CPU and memory use from crafted databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws - `InvalidDatabaseException`. + `InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for details. * Oversized integer encodings are rejected. * Truncated payloads and malformed UTF-8 are rejected. diff --git a/UPGRADING.md b/UPGRADING.md index 3feee79d..2209552d 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,3 +1,33 @@ +# Upgrading to 4.2.0 + +## Decoder Resource Limits + +Version 4.2.0 limits the work and memory used by one record or metadata decode. +The decoder rejects an operation that exceeds any of these limits: + +- 65,536 decoded or skipped values under the Java reader's work accounting +- 128 nested maps or arrays +- 2 MiB of encoded string and bytes payload materialized by the decoder + +Each cached pointer target retains its logical value, depth, and payload cost. +Every decoded pointer occurrence consumes that recorded cost. The cache still +avoids decoding or materializing the target again, but cache state does not +determine whether an operation exceeds a limit. A pointer in a field the +decoder skips counts as one value. The decoder does not visit its target. + +These limits leave a wide margin above MaxMind-produced records. A custom +database containing an unusually large record that decoded in an earlier +release may now throw `InvalidDatabaseException`. The limits are not +configurable in this release. + +When the decoder constructs a custom `List` or `Map` type through an `int` +constructor, it passes an initial-capacity hint capped at 128 rather than the +full declared collection size. + +The decoder also rejects a data-section pointer whose target is another pointer, +which the MaxMind DB format does not permit. It rejects integer payloads wider +than their format type permits before reading the payload. + # Upgrading to 4.0.0 This guide covers the breaking changes introduced in version 4.0.0 and how to diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index f54497b2..a692cc4d 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -56,7 +56,7 @@ class Decoder { // A collection's declared size is its logical child count, but it is not // proof that the input contains that many decodable children. When deriving // an initial capacity from it, limit unused capacity on the active recursion - // path; completed children remain bounded by MAX_VALUES. + // path. Completed children remain bounded by MAX_VALUES. private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; private int maxDepth; From 620924d02252d6b8ebc038a1d0566a5a40f6918e Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 22:25:37 +0000 Subject: [PATCH 12/25] Reduce decoder allocation overhead Return raw values within the decoder and create DecodedValue wrappers only at cache boundaries. Reuse the thread-local UTF-8 decoder, remove the per-decoder cache-loader lambda, and short-circuit built-in collection targets. --- CHANGELOG.md | 1 + .../java/com/maxmind/db/DecodedValue.java | 44 +++--- src/main/java/com/maxmind/db/Decoder.java | 135 ++++++++++-------- src/test/java/com/maxmind/db/DecoderTest.java | 2 +- src/test/java/com/maxmind/db/PointerTest.java | 2 +- src/test/java/com/maxmind/db/TestDecoder.java | 4 +- 6 files changed, 104 insertions(+), 84 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e39618c6..aeb83732 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -19,6 +19,7 @@ CHANGELOG `InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for details. * Oversized integer encodings are rejected. * Truncated payloads and malformed UTF-8 are rejected. +* Improved decoder performance and reduced per-lookup allocation. 4.1.0 (2026-05-12) ------------------ diff --git a/src/main/java/com/maxmind/db/DecodedValue.java b/src/main/java/com/maxmind/db/DecodedValue.java index 7101186d..a3f37eb1 100644 --- a/src/main/java/com/maxmind/db/DecodedValue.java +++ b/src/main/java/com/maxmind/db/DecodedValue.java @@ -8,46 +8,48 @@ public final class DecodedValue { private static final int VALUES_SHIFT = 30; private static final long PAYLOAD_MASK = (1L << 22) - 1; - Object value; + final Object value; + // A NodeCache is user-supplied and may publish this instance to another + // thread without a happens-before edge. Set the costs in the constructor + // so a reader cannot see a zero budget charge for a non-empty value. + private final long costs; - DecodedValue(Object value) { + DecodedValue(Object value, int values, long payloadBytes, int depth) { this.value = value; + this.costs = ((long) values << VALUES_SHIFT) + | (payloadBytes << PAYLOAD_SHIFT) + | depth; } Object value() { - if (value instanceof CostedValue costedValue) { - return costedValue.value(); - } return value; } int values() { - return (int) (costs() >>> VALUES_SHIFT); + return values(costs()); + } + + static int values(long costs) { + return (int) (costs >>> VALUES_SHIFT); } long payloadBytes() { - return (costs() >>> PAYLOAD_SHIFT) & PAYLOAD_MASK; + return payloadBytes(costs()); } - int depth() { - return (int) (costs() & 0xFF); + static long payloadBytes(long costs) { + return (costs >>> PAYLOAD_SHIFT) & PAYLOAD_MASK; } - DecodedValue costs(int values, long payloadBytes, int depth) { - var costs = ((long) values << VALUES_SHIFT) - | (payloadBytes << PAYLOAD_SHIFT) - | depth; - this.value = new CostedValue(this.value, costs); - return this; + int depth() { + return depth(costs()); } - private long costs() { - if (value instanceof CostedValue costedValue) { - return costedValue.costs(); - } - return 0; + static int depth(long costs) { + return (int) (costs & 0xFF); } - private record CostedValue(Object value, long costs) { + long costs() { + return this.costs; } } diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index a692cc4d..078af9c8 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -24,9 +24,11 @@ * * This class CANNOT be shared between threads */ -class Decoder { +class Decoder implements NodeCache.Loader { private static final Charset UTF_8 = StandardCharsets.UTF_8; + private static final ThreadLocal UTF_8_DECODER = + ThreadLocal.withInitial(UTF_8::newDecoder); private static final int[] POINTER_VALUE_OFFSETS = {0, 0, 1 << 11, (1 << 19) + (1 << 11), 0}; @@ -59,13 +61,13 @@ class Decoder { // path. Completed children remain bounded by MAX_VALUES. private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; - private int maxDepth; + private int maxDepth = -1; private int valuesRemaining = MAX_VALUES; private long payloadRemaining = MAX_PAYLOAD_BYTES; private final long pointerBase; - private final CharsetDecoder utfDecoder = UTF_8.newDecoder(); + private final CharsetDecoder utfDecoder = UTF_8_DECODER.get(); private final Buffer buffer; @@ -123,8 +125,6 @@ class Decoder { this.lookupNetwork = lookupNetwork; } - private final NodeCache.Loader cacheLoader = this::decodeForCache; - T decode(long offset, Class cls) throws IOException { if (offset >= this.buffer.capacity()) { throw new InvalidDatabaseException( @@ -135,32 +135,13 @@ T decode(long offset, Class cls) throws IOException { this.valuesRemaining = MAX_VALUES; this.payloadRemaining = MAX_PAYLOAD_BYTES; this.depth = 0; - this.maxDepth = 0; + this.maxDepth = -1; + this.utfDecoder.reset(); this.buffer.position(offset); - return cls.cast(decode(cls, null).value()); + return cls.cast(decode(cls, null)); } - private DecodedValue decode(CacheKey key) throws IOException { - long offset = key.offset(); - if (offset >= this.buffer.capacity()) { - throw new InvalidDatabaseException( - "The MaxMind DB file's data section contains bad data: " - + "pointer larger than the database."); - } - // Validate a target when the cache loader decodes it. A target that was - // loaded successfully has already passed this check, so cache hits do - // not need to reread its control byte. - if (Type.fromControlByte(0xFF & this.buffer.get(offset)) == Type.POINTER) { - throw new InvalidDatabaseException( - "The MaxMind DB file's data section contains a pointer to a pointer"); - } - - this.buffer.position(offset); - Class cls = key.cls(); - return decode(cls, key.type()); - } - - private DecodedValue decode(Class cls, java.lang.reflect.Type genericType) + private Object decode(Class cls, java.lang.reflect.Type genericType) throws IOException { if (--this.valuesRemaining < 0) { throw new InvalidDatabaseException( @@ -206,18 +187,40 @@ private DecodedValue decode(Class cls, java.lang.reflect.Type genericType }; } - return new DecodedValue(this.decodeByType(type, size, cls, genericType)); + return this.decodeByType(type, size, cls, genericType); + } + + private Object decodeTarget(CacheKey key) throws IOException { + long offset = key.offset(); + if (offset >= this.buffer.capacity()) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains bad data: " + + "pointer larger than the database."); + } + // Validate a target when the cache loader decodes it. A target that was + // loaded successfully has already passed this check, so cache hits do + // not need to reread its control byte. + if (Type.fromControlByte(0xFF & this.buffer.get(offset)) == Type.POINTER) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains a pointer to a pointer"); + } + + this.buffer.position(offset); + Class cls = key.cls(); + return decode(cls, key.type()); } - private DecodedValue decodeForCache(CacheKey key) throws IOException { + @Override + public DecodedValue load(CacheKey key) throws IOException { var valuesRemaining = this.valuesRemaining; var payloadRemaining = this.payloadRemaining; var depth = this.depth; var maxDepth = this.maxDepth; this.maxDepth = depth; try { - var value = this.decode(key); - return value.costs( + var value = this.decodeTarget(key); + return new DecodedValue( + value, valuesRemaining - this.valuesRemaining, payloadRemaining - this.payloadRemaining, this.maxDepth - depth @@ -230,17 +233,18 @@ private DecodedValue decodeForCache(CacheKey key) throws IOException { } } - DecodedValue decodePointer(long pointer, Class cls, java.lang.reflect.Type genericType) + Object decodePointer(long pointer, Class cls, java.lang.reflect.Type genericType) throws IOException { var position = buffer.position(); var key = new CacheKey<>(pointer, cls, genericType); - DecodedValue value; + Object value; if (this.cache == NoCache.getInstance() || requiresLookupContext(cls)) { - value = this.decode(key); + value = this.decodeTarget(key); } else { - value = cache.get(key, cacheLoader); - this.charge(value); + var decodedValue = cache.get(key, this); + this.charge(decodedValue); + value = decodedValue.value(); } buffer.position(position); @@ -248,27 +252,36 @@ DecodedValue decodePointer(long pointer, Class cls, java.lang.reflect.Type ge } private void charge(DecodedValue value) throws InvalidDatabaseException { - if (value.values() > this.valuesRemaining) { + var costs = value.costs(); + var values = DecodedValue.values(costs); + var payloadBytes = DecodedValue.payloadBytes(costs); + var depth = DecodedValue.depth(costs); + var valuesRemaining = this.valuesRemaining - values; + var payloadRemaining = this.payloadRemaining - payloadBytes; + + if (valuesRemaining < 0) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum number of values"); } - if (value.payloadBytes() > this.payloadRemaining) { + if (payloadRemaining < 0) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum payload size"); } - if (value.depth() > MAX_DEPTH - this.depth) { + if (depth > MAX_DEPTH - this.depth) { throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum depth"); } - this.valuesRemaining -= value.values(); - this.payloadRemaining -= value.payloadBytes(); - this.maxDepth = Math.max(this.maxDepth, this.depth + value.depth()); + this.valuesRemaining = valuesRemaining; + this.payloadRemaining = payloadRemaining; + if (this.maxDepth >= 0) { + this.maxDepth = Math.max(this.maxDepth, this.depth + depth); + } } private boolean requiresLookupContext(Class cls) { if (cls == null - || cls.equals(Object.class) + || cls == Object.class || Map.class.isAssignableFrom(cls) || List.class.isAssignableFrom(cls) || cls.isEnum() @@ -293,11 +306,11 @@ private static boolean isSimpleType(Class cls) { if (cls.isPrimitive() || cls.isArray()) { return true; } - return cls.equals(String.class) + return cls == String.class || Number.class.isAssignableFrom(cls) - || cls.equals(Boolean.class) - || cls.equals(Character.class) - || cls.equals(BigInteger.class); + || cls == Boolean.class + || cls == Character.class + || cls == BigInteger.class; } // A container cannot hold more entries than there are bytes left to encode @@ -328,7 +341,9 @@ private void enterContainer(long valueCount) throws InvalidDatabaseException { "The MaxMind DB file's data section exceeds the maximum depth"); } this.depth++; - this.maxDepth = Math.max(this.maxDepth, this.depth); + if (this.maxDepth >= 0) { + this.maxDepth = Math.max(this.maxDepth, this.depth); + } } // Charge a string or bytes payload against the per-operation budget before @@ -648,13 +663,15 @@ private List decodeArray( Class cls, Class elementClass ) throws IOException { - if (!List.class.isAssignableFrom(cls) && !cls.equals(Object.class)) { + if (cls != Object.class + && cls != List.class + && !List.class.isAssignableFrom(cls)) { throw new DeserializationException("Unable to deserialize an array into an " + cls); } List array; var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); - if (cls.equals(List.class) || cls.equals(Object.class)) { + if (cls == List.class || cls == Object.class) { array = new ArrayList<>(initialCapacity); } else { Constructor constructor; @@ -677,7 +694,7 @@ private List decodeArray( } for (int i = 0; i < size; i++) { - var e = this.decode(elementClass, null).value(); + var e = this.decode(elementClass, null); array.add(elementClass.cast(e)); } @@ -689,13 +706,13 @@ private Object decodeMap( Class cls, java.lang.reflect.Type genericType ) throws IOException { - if (Map.class.isAssignableFrom(cls) || cls.equals(Object.class)) { + if (cls == Object.class || cls == Map.class || Map.class.isAssignableFrom(cls)) { Class valueClass = Object.class; if (genericType instanceof ParameterizedType ptype) { var actualTypes = ptype.getActualTypeArguments(); if (actualTypes.length == 2) { var keyClass = (Class) actualTypes[0]; - if (!keyClass.equals(String.class)) { + if (keyClass != String.class) { throw new DeserializationException("Map keys must be strings."); } @@ -714,7 +731,7 @@ private Map decodeMapIntoMap( Class valueClass ) throws IOException { Map map; - if (cls.equals(Map.class) || cls.equals(Object.class)) { + if (cls == Map.class || cls == Object.class) { map = new HashMap<>(initialMapCapacity(size)); } else { var initialCapacity = Math.min(size, MAX_INITIAL_COLLECTION_CAPACITY); @@ -738,8 +755,8 @@ private Map decodeMapIntoMap( } for (int i = 0; i < size; i++) { - var key = (String) this.decode(String.class, null).value(); - var value = this.decode(valueClass, null).value(); + var key = (String) this.decode(String.class, null); + var value = this.decode(valueClass, null); try { map.put(key, valueClass.cast(value)); } catch (ClassCastException e) { @@ -850,7 +867,7 @@ private Object decodeMapIntoObject(int size, Class cls) var parameters = new Object[parameterTypes.length]; for (int i = 0; i < size; i++) { - var key = (String) this.decode(String.class, null).value(); + var key = (String) this.decode(String.class, null); var parameterIndex = parameterIndexes.get(key); if (parameterIndex == null) { @@ -862,7 +879,7 @@ private Object decodeMapIntoObject(int size, Class cls) parameters[parameterIndex] = this.decode( parameterTypes[parameterIndex], parameterGenericTypes[parameterIndex] - ).value(); + ); } for (int i = 0; i < parameters.length; i++) { diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 0db22081..a4466423 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -25,7 +25,7 @@ public class DecoderTest { @Test public void testDecodedValueStoresMaximumCosts() { - var value = new DecodedValue(null).costs(1 << 16, 1L << 21, TEST_MAX_DEPTH); + var value = new DecodedValue(null, 1 << 16, 1L << 21, TEST_MAX_DEPTH); assertEquals(1 << 16, value.values()); assertEquals(1L << 21, value.payloadBytes()); assertEquals(TEST_MAX_DEPTH, value.depth()); diff --git a/src/test/java/com/maxmind/db/PointerTest.java b/src/test/java/com/maxmind/db/PointerTest.java index 23ab6c19..4e34cfd2 100644 --- a/src/test/java/com/maxmind/db/PointerTest.java +++ b/src/test/java/com/maxmind/db/PointerTest.java @@ -54,7 +54,7 @@ public void testPointerBeyond2GiBIsNotSignExtended() throws IOException { var observed = new AtomicLong(Long.MIN_VALUE); NodeCache cache = (key, loader) -> { observed.set(key.offset()); - return new DecodedValue(null); + return new DecodedValue(null, 0, 0, 0); }; new Decoder(cache, buffer, 0).decode(0, Object.class); diff --git a/src/test/java/com/maxmind/db/TestDecoder.java b/src/test/java/com/maxmind/db/TestDecoder.java index 99bf9896..bbc74c51 100644 --- a/src/test/java/com/maxmind/db/TestDecoder.java +++ b/src/test/java/com/maxmind/db/TestDecoder.java @@ -10,9 +10,9 @@ final class TestDecoder extends Decoder { } @Override - DecodedValue decodePointer(long pointer, Class cls, Type genericType) { + Object decodePointer(long pointer, Class cls, Type genericType) { // bypass cache - return new DecodedValue(pointer); + return pointer; } } From 74ccdb5e77b1de5cdde36bd23caa8d9b60e1a900 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Fri, 4 Sep 2026 22:40:31 +0000 Subject: [PATCH 13/25] Fix UTF-8 decoding across buffer chunks Use the decoder's one-shot API for strings contained in one chunk. Copy only bounded strings that cross chunks so incomplete UTF-8 sequences remain intact and end-of-input validation runs. The one-shot API manages decoder state, so the top-level manual reset is no longer needed. --- CHANGELOG.md | 2 + src/main/java/com/maxmind/db/Decoder.java | 1 - src/main/java/com/maxmind/db/MultiBuffer.java | 58 +++++++++---------- .../java/com/maxmind/db/MultiBufferTest.java | 25 ++++++++ 4 files changed, 53 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aeb83732..9776b6ba 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,8 @@ CHANGELOG * Fixed skipping unknown four-byte pointers during typed decoding. Skipped values that extend past the data section are now rejected. MaxMind-produced databases were unaffected. +* Fixed UTF-8 decoding across buffer chunks and rejection of incomplete + multibyte characters at the end of a string. * Added decoder limits to prevent excessive CPU and memory use from crafted databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 078af9c8..32d75a46 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -136,7 +136,6 @@ T decode(long offset, Class cls) throws IOException { this.payloadRemaining = MAX_PAYLOAD_BYTES; this.depth = 0; this.maxDepth = -1; - this.utfDecoder.reset(); this.buffer.position(offset); return cls.cast(decode(cls, null)); } diff --git a/src/main/java/com/maxmind/db/MultiBuffer.java b/src/main/java/com/maxmind/db/MultiBuffer.java index 16a02014..39f1136e 100644 --- a/src/main/java/com/maxmind/db/MultiBuffer.java +++ b/src/main/java/com/maxmind/db/MultiBuffer.java @@ -2,7 +2,6 @@ import java.io.IOException; import java.nio.ByteBuffer; -import java.nio.CharBuffer; import java.nio.channels.FileChannel; import java.nio.charset.CharacterCodingException; import java.nio.charset.CharsetDecoder; @@ -215,48 +214,43 @@ public String decode(CharsetDecoder decoder) return this.decode(decoder, Integer.MAX_VALUE); } - String decode(CharsetDecoder decoder, int maxCharBufferSize) + String decode(CharsetDecoder decoder, int maximumSize) throws CharacterCodingException { var remainingBytes = limit - position; - // Cannot allocate more than maxCharBufferSize for CharBuffer - if (remainingBytes > maxCharBufferSize) { + if (remainingBytes > maximumSize) { throw new IllegalStateException( - "Decoding region too large to fit in a CharBuffer: " + remainingBytes + "Decoding region exceeds the maximum size: " + remainingBytes ); } - var out = CharBuffer.allocate((int) remainingBytes); - var pos = position; - - while (remainingBytes > 0) { - // Locate which underlying buffer we are in - var bufIndex = (int) (pos / this.chunkSize); - var bufOffset = (int) (pos % this.chunkSize); - - var srcView = buffers[bufIndex]; - var savedLimit = srcView.limit(); - srcView.position(bufOffset); - - var toRead = (int) Math.min(srcView.remaining(), remainingBytes); - srcView.limit(bufOffset + toRead); - - var result = decoder.decode(srcView, out, false); - srcView.limit(savedLimit); + if (remainingBytes == 0) { + return ""; + } - if (result.isError()) { - result.throwException(); + var bufIndex = (int) (position / this.chunkSize); + var bufOffset = (int) (position % this.chunkSize); + var source = buffers[bufIndex]; + if (remainingBytes <= source.limit() - bufOffset) { + var savedLimit = source.limit(); + source.position(bufOffset); + source.limit(bufOffset + (int) remainingBytes); + try { + var value = decoder.decode(source).toString(); + this.position += remainingBytes; + return value; + } finally { + source.limit(savedLimit); } - - pos += toRead; - remainingBytes -= toRead; } - // Update this MultiBuffer’s logical position - this.position = pos; - - out.flip(); - return out.toString(); + var bytes = new byte[(int) remainingBytes]; + var savedPosition = this.position; + this.get(bytes); + this.position = savedPosition; + var value = decoder.decode(ByteBuffer.wrap(bytes)).toString(); + this.position = this.limit; + return value; } /** diff --git a/src/test/java/com/maxmind/db/MultiBufferTest.java b/src/test/java/com/maxmind/db/MultiBufferTest.java index 43be354f..4bd339ed 100644 --- a/src/test/java/com/maxmind/db/MultiBufferTest.java +++ b/src/test/java/com/maxmind/db/MultiBufferTest.java @@ -370,4 +370,29 @@ public void testDecodeAcrossChunks() throws CharacterCodingException { assertEquals("123456789012345678901234567", result); assertEquals(89, buffer.position()); } + + @Test + public void testDecodeMultibyteCharacterAcrossChunks() throws CharacterCodingException { + var bytes = "a€b".getBytes(StandardCharsets.UTF_8); + var chunks = new ByteBuffer[]{ + ByteBuffer.wrap(new byte[]{bytes[0], bytes[1]}), + ByteBuffer.wrap(new byte[]{bytes[2], bytes[3]}), + ByteBuffer.wrap(new byte[]{bytes[4]}) + }; + var buffer = new MultiBuffer(chunks, 2); + + assertEquals("a€b", buffer.decode(StandardCharsets.UTF_8.newDecoder())); + assertEquals(bytes.length, buffer.position()); + } + + @Test + public void testDecodeRejectsIncompleteCharacter() { + var bytes = new byte[]{'a', (byte) 0xe2}; + var buffer = new MultiBuffer(new ByteBuffer[]{ByteBuffer.wrap(bytes)}, bytes.length); + + assertThrows( + CharacterCodingException.class, + () -> buffer.decode(StandardCharsets.UTF_8.newDecoder()) + ); + } } From 92f710dc2f4972070b1a1e55cf820a47de293832 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Sat, 5 Sep 2026 20:56:28 +0000 Subject: [PATCH 14/25] Speed up UTF-8 string decoding --- CHANGELOG.md | 3 +- src/main/java/com/maxmind/db/Buffer.java | 12 ---- src/main/java/com/maxmind/db/Decoder.java | 32 +++++---- src/main/java/com/maxmind/db/MultiBuffer.java | 48 ------------- .../java/com/maxmind/db/SingleBuffer.java | 9 --- src/test/java/com/maxmind/db/DecoderTest.java | 68 +++++++++++++++++++ .../java/com/maxmind/db/MultiBufferTest.java | 55 --------------- 7 files changed, 88 insertions(+), 139 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9776b6ba..66b59487 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,8 @@ CHANGELOG `InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for details. * Oversized integer encodings are rejected. * Truncated payloads and malformed UTF-8 are rejected. -* Improved decoder performance and reduced per-lookup allocation. +* Improved decoder performance and reduced per-lookup allocation, including + UTF-8 string decoding. 4.1.0 (2026-05-12) ------------------ diff --git a/src/main/java/com/maxmind/db/Buffer.java b/src/main/java/com/maxmind/db/Buffer.java index b12dbda7..b9c8f206 100644 --- a/src/main/java/com/maxmind/db/Buffer.java +++ b/src/main/java/com/maxmind/db/Buffer.java @@ -1,8 +1,5 @@ package com.maxmind.db; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.CharsetDecoder; - /** * A generic buffer abstraction that supports sequential and random access * to binary data. Implementations may be backed by a single {@link @@ -96,13 +93,4 @@ sealed interface Buffer permits SingleBuffer, MultiBuffer { * @return a duplicate buffer */ Buffer duplicate(); - - /** - * Decodes the buffer's content into a string using the given decoder. - * - * @param decoder the charset decoder - * @return the decoded string - * @throws CharacterCodingException if decoding fails - */ - String decode(CharsetDecoder decoder) throws CharacterCodingException; } diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 32d75a46..7af4d8f0 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -9,9 +9,9 @@ import java.lang.reflect.ParameterizedType; import java.math.BigInteger; import java.net.InetAddress; +import java.nio.ByteBuffer; import java.nio.charset.CharacterCodingException; import java.nio.charset.Charset; -import java.nio.charset.CharsetDecoder; import java.nio.charset.StandardCharsets; import java.util.ArrayList; import java.util.HashMap; @@ -27,8 +27,6 @@ class Decoder implements NodeCache.Loader { private static final Charset UTF_8 = StandardCharsets.UTF_8; - private static final ThreadLocal UTF_8_DECODER = - ThreadLocal.withInitial(UTF_8::newDecoder); private static final int[] POINTER_VALUE_OFFSETS = {0, 0, 1 << 11, (1 << 19) + (1 << 11), 0}; @@ -67,8 +65,6 @@ class Decoder implements NodeCache.Loader { private final long pointerBase; - private final CharsetDecoder utfDecoder = UTF_8_DECODER.get(); - private final Buffer buffer; private final ConcurrentHashMap, CachedConstructor> constructors; @@ -571,16 +567,24 @@ private static Object coerceFromBigInteger(BigInteger value, Class target) { private String decodeString(long size) throws IOException { this.chargePayload(size); - var oldLimit = buffer.limit(); - try { - buffer.limit(buffer.position() + size); - return buffer.decode(utfDecoder); - } catch (CharacterCodingException e) { - throw new InvalidDatabaseException( - "The MaxMind DB file's data section contains an invalid UTF-8 string", e); - } finally { - buffer.limit(oldLimit); + // Performance optimization: String's UTF-8 path avoids the temporary + // CharBuffer and char[] used by CharsetDecoder, despite this byte[] copy. + // On OpenJDK 26, random GeoLite2-City lookup throughput improved by about + // 6% with CHMCache and 22% without caching over the previous decoder. + var bytes = new byte[(int) size]; + this.buffer.get(bytes); + var value = new String(bytes, UTF_8); + // String replaces malformed UTF-8 with U+FFFD. Validate strings containing + // that character to distinguish malformed input from a literal U+FFFD. + if (value.indexOf(0xFFFD) >= 0) { + try { + UTF_8.newDecoder().decode(ByteBuffer.wrap(bytes)); + } catch (CharacterCodingException e) { + throw new InvalidDatabaseException( + "The MaxMind DB file's data section contains an invalid UTF-8 string", e); + } } + return value; } private int decodeUint16(int size) { diff --git a/src/main/java/com/maxmind/db/MultiBuffer.java b/src/main/java/com/maxmind/db/MultiBuffer.java index 39f1136e..4fbb167b 100644 --- a/src/main/java/com/maxmind/db/MultiBuffer.java +++ b/src/main/java/com/maxmind/db/MultiBuffer.java @@ -3,8 +3,6 @@ import java.io.IOException; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.CharsetDecoder; /** * A {@link Buffer} implementation backed by multiple {@link ByteBuffer}s, @@ -207,52 +205,6 @@ public Buffer duplicate() { return copy; } - /** {@inheritDoc} */ - @Override - public String decode(CharsetDecoder decoder) - throws CharacterCodingException { - return this.decode(decoder, Integer.MAX_VALUE); - } - - String decode(CharsetDecoder decoder, int maximumSize) - throws CharacterCodingException { - var remainingBytes = limit - position; - - if (remainingBytes > maximumSize) { - throw new IllegalStateException( - "Decoding region exceeds the maximum size: " + remainingBytes - ); - } - - if (remainingBytes == 0) { - return ""; - } - - var bufIndex = (int) (position / this.chunkSize); - var bufOffset = (int) (position % this.chunkSize); - var source = buffers[bufIndex]; - if (remainingBytes <= source.limit() - bufOffset) { - var savedLimit = source.limit(); - source.position(bufOffset); - source.limit(bufOffset + (int) remainingBytes); - try { - var value = decoder.decode(source).toString(); - this.position += remainingBytes; - return value; - } finally { - source.limit(savedLimit); - } - } - - var bytes = new byte[(int) remainingBytes]; - var savedPosition = this.position; - this.get(bytes); - this.position = savedPosition; - var value = decoder.decode(ByteBuffer.wrap(bytes)).toString(); - this.position = this.limit; - return value; - } - /** * Creates a read-only {@code MultiBuffer} by memory-mapping the given * {@link FileChannel}. diff --git a/src/main/java/com/maxmind/db/SingleBuffer.java b/src/main/java/com/maxmind/db/SingleBuffer.java index eca1629a..09a956f8 100644 --- a/src/main/java/com/maxmind/db/SingleBuffer.java +++ b/src/main/java/com/maxmind/db/SingleBuffer.java @@ -4,8 +4,6 @@ import java.nio.ByteBuffer; import java.nio.channels.FileChannel; import java.nio.channels.FileChannel.MapMode; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.CharsetDecoder; /** * A {@link Buffer} implementation backed by a single {@link ByteBuffer}. @@ -99,13 +97,6 @@ public SingleBuffer duplicate() { return new SingleBuffer(this.buffer.duplicate()); } - /** {@inheritDoc} */ - @Override - public String decode(CharsetDecoder decoder) - throws CharacterCodingException { - return decoder.decode(buffer).toString(); - } - /** * Wraps the given byte array in a new {@code SingleBuffer}. * diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index a4466423..855503a5 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -4,11 +4,14 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertInstanceOf; import static org.junit.jupiter.api.Assertions.assertThrows; import java.io.ByteArrayOutputStream; import java.io.IOException; import java.math.BigInteger; +import java.nio.ByteBuffer; +import java.nio.charset.CharacterCodingException; import java.nio.charset.StandardCharsets; import java.nio.file.Path; import java.util.ArrayList; @@ -149,6 +152,8 @@ private static Map strings() { DecoderTest.addTestString(strings, (byte) 0x40, ""); DecoderTest.addTestString(strings, (byte) 0x41, "1"); DecoderTest.addTestString(strings, (byte) 0x43, "人"); + DecoderTest.addTestString(strings, (byte) 0x43, "\uFFFD"); + DecoderTest.addTestString(strings, (byte) 0x45, "a\uFFFDz"); DecoderTest.addTestString(strings, (byte) 0x43, "123"); DecoderTest.addTestString(strings, (byte) 0x5b, "123456789012345678901234567"); @@ -488,6 +493,69 @@ public void testStrings() throws IOException { DecoderTest.strings()); } + @Test + public void testUtf8PointerAcrossChunks() throws IOException { + var expected = "a€𐍈\uFFFDz"; + var payload = expected.getBytes(StandardCharsets.UTF_8); + for (int chunkSize : new int[] {1, 2, 3, 4, 5, 64}) { + for (var cache : List.of(NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + var decoder = stringPointerDecoder(payload, chunkSize, cache); + assertEquals(expected, decoder.decode(0, String.class)); + assertEquals(expected, decoder.decode(0, String.class)); + assertEquals("a", decoder.decode(payload.length + 3, String.class)); + } + } + } + + @Test + public void testMalformedUtf8IsRejectedAcrossChunks() throws IOException { + var payloads = List.of( + new byte[] {(byte) 0x80}, + new byte[] {(byte) 0xC0, (byte) 0xAF}, + new byte[] {(byte) 0xC2}, + new byte[] {(byte) 0xE2, (byte) 0x82}, + new byte[] {(byte) 0xED, (byte) 0xA0, (byte) 0x80}, + new byte[] {(byte) 0xF0, (byte) 0x9F, (byte) 0x92}, + new byte[] {(byte) 0xF4, (byte) 0x90, (byte) 0x80, (byte) 0x80}, + new byte[] {(byte) 0xFF}, + new byte[] {(byte) 0xEF, (byte) 0xBF, (byte) 0xBD, (byte) 0xFF} + ); + for (var payload : payloads) { + for (int chunkSize : new int[] {1, 2, 3, 4, 5, 64}) { + for (var cache : List.of(NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + var decoder = stringPointerDecoder(payload, chunkSize, cache); + var error = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, String.class) + ); + assertInstanceOf(CharacterCodingException.class, error.getCause()); + assertEquals("a", decoder.decode(payload.length + 3, String.class)); + assertThrows(InvalidDatabaseException.class, () -> decoder.decode(0, String.class)); + } + } + } + } + + private static Decoder stringPointerDecoder(byte[] payload, int chunkSize, NodeCache cache) { + var data = new byte[payload.length + 5]; + data[0] = 0x20; + data[1] = 2; + data[2] = (byte) (0x40 | payload.length); + System.arraycopy(payload, 0, data, 3, payload.length); + data[data.length - 2] = 0x41; + data[data.length - 1] = 'a'; + if (chunkSize >= data.length) { + return new Decoder(cache, SingleBuffer.wrap(data), 0); + } + var chunks = new ByteBuffer[(data.length + chunkSize - 1) / chunkSize]; + for (int i = 0; i < chunks.length; i++) { + int offset = i * chunkSize; + int size = Math.min(chunkSize, data.length - offset); + chunks[i] = ByteBuffer.wrap(data, offset, size).slice(); + } + return new Decoder(cache, new MultiBuffer(chunks, chunkSize), 0); + } + @Test public void testBooleans() throws IOException { DecoderTest.testTypeDecoding(Type.BOOLEAN, diff --git a/src/test/java/com/maxmind/db/MultiBufferTest.java b/src/test/java/com/maxmind/db/MultiBufferTest.java index 4bd339ed..2e38e6f8 100644 --- a/src/test/java/com/maxmind/db/MultiBufferTest.java +++ b/src/test/java/com/maxmind/db/MultiBufferTest.java @@ -6,8 +6,6 @@ import java.io.RandomAccessFile; import java.nio.ByteBuffer; import java.nio.channels.FileChannel; -import java.nio.charset.CharacterCodingException; -import java.nio.charset.StandardCharsets; import java.nio.file.Files; import java.nio.file.Path; import java.nio.file.StandardOpenOption; @@ -342,57 +340,4 @@ public void testMapFromEmptyChannel(@TempDir Path tempDir) throws IOException { } } - @Test - public void testDecodeString() throws CharacterCodingException { - var buffer = createBuffer(22); - buffer.position(26); - buffer.limit(29); - var result = buffer.decode(StandardCharsets.UTF_8.newDecoder()); - assertEquals("123", result); - assertEquals(29, buffer.position()); - } - - @Test - public void testDecodeStringTooLarge() { - var buffer = createBuffer(65); - buffer.position(62); - buffer.limit(89); - assertThrows(IllegalStateException.class, () -> - buffer.decode(StandardCharsets.UTF_8.newDecoder(), 20)); - } - - @Test - public void testDecodeAcrossChunks() throws CharacterCodingException { - var buffer = createBuffer(65); - buffer.position(62); - buffer.limit(89); - var result = buffer.decode(StandardCharsets.UTF_8.newDecoder()); - assertEquals("123456789012345678901234567", result); - assertEquals(89, buffer.position()); - } - - @Test - public void testDecodeMultibyteCharacterAcrossChunks() throws CharacterCodingException { - var bytes = "a€b".getBytes(StandardCharsets.UTF_8); - var chunks = new ByteBuffer[]{ - ByteBuffer.wrap(new byte[]{bytes[0], bytes[1]}), - ByteBuffer.wrap(new byte[]{bytes[2], bytes[3]}), - ByteBuffer.wrap(new byte[]{bytes[4]}) - }; - var buffer = new MultiBuffer(chunks, 2); - - assertEquals("a€b", buffer.decode(StandardCharsets.UTF_8.newDecoder())); - assertEquals(bytes.length, buffer.position()); - } - - @Test - public void testDecodeRejectsIncompleteCharacter() { - var bytes = new byte[]{'a', (byte) 0xe2}; - var buffer = new MultiBuffer(new ByteBuffer[]{ByteBuffer.wrap(bytes)}, bytes.length); - - assertThrows( - CharacterCodingException.class, - () -> buffer.decode(StandardCharsets.UTF_8.newDecoder()) - ); - } } From d1e6dc4ecb6bc59f6e7965166b860ae77bf89e70 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 15:25:08 +0000 Subject: [PATCH 15/25] Reject truncated value headers with InvalidDatabaseException The decoder read a control byte, an extended type byte, a size header, a pointer, a double, or a float without checking that the bytes are inside the data section. A database truncated inside one of these values threw BufferUnderflowException from SingleBuffer or IndexOutOfBoundsException from MultiBuffer, which callers do not expect. Call checkDataSize before each read, as the string, bytes, and integer paths already do. The control-byte check runs once per decoded value, so the decoder holds the buffer capacity in a field rather than calling Buffer.capacity() on each check. On OpenJDK 26, random GeoLite2-City lookups measured within 1% of the previous throughput with the field and about 4% below it without. Co-Authored-By: Claude Opus 5 (1M context) --- CHANGELOG.md | 5 ++ src/main/java/com/maxmind/db/Decoder.java | 26 +++++-- src/test/java/com/maxmind/db/DecoderTest.java | 68 +++++++++++++++++++ 3 files changed, 93 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 66b59487..325dc178 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,11 @@ CHANGELOG databases were unaffected. * Fixed UTF-8 decoding across buffer chunks and rejection of incomplete multibyte characters at the end of a string. +* Fixed the exception thrown for a database truncated in the middle of a + value. Reading a control byte, an extended type byte, a size header, a + pointer, a `double`, or a `float` past the end of the data section threw + `BufferUnderflowException` or `IndexOutOfBoundsException`. These now throw + `InvalidDatabaseException`, as the rest of the reader does. * Added decoder limits to prevent excessive CPU and memory use from crafted databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 7af4d8f0..0074de10 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -67,6 +67,8 @@ class Decoder implements NodeCache.Loader { private final Buffer buffer; + private final long capacity; + private final ConcurrentHashMap, CachedConstructor> constructors; private final ConcurrentHashMap, CachedCreator> creators; @@ -115,6 +117,9 @@ class Decoder implements NodeCache.Loader { this.cache = cache; this.pointerBase = pointerBase; this.buffer = buffer; + // The bounds checks run once per decoded value, so read the fixed + // capacity here rather than through the Buffer interface each time. + this.capacity = buffer.capacity(); this.constructors = constructors; this.creators = creators; this.lookupIp = lookupIp; @@ -122,7 +127,7 @@ class Decoder implements NodeCache.Loader { } T decode(long offset, Class cls) throws IOException { - if (offset >= this.buffer.capacity()) { + if (offset >= this.capacity) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); @@ -142,6 +147,7 @@ private Object decode(Class cls, java.lang.reflect.Type genericType) throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum number of values"); } + this.checkDataSize(1); var ctrlByte = 0xFF & this.buffer.get(); var type = Type.fromControlByte(ctrlByte); @@ -151,6 +157,7 @@ private Object decode(Class cls, java.lang.reflect.Type genericType) // it. if (type.equals(Type.POINTER)) { var pointerSize = ((ctrlByte >>> 3) & 0x3) + 1; + this.checkDataSize(pointerSize); var base = pointerSize == 4 ? (byte) 0 : (byte) (ctrlByte & 0x7); var packed = Decoder.decodeLong(this.buffer, base, pointerSize); var pointer = packed + this.pointerBase + POINTER_VALUE_OFFSETS[pointerSize]; @@ -159,6 +166,7 @@ private Object decode(Class cls, java.lang.reflect.Type genericType) } if (type.equals(Type.EXTENDED)) { + this.checkDataSize(1); var nextByte = this.buffer.get(); var typeNum = nextByte + 7; @@ -175,6 +183,8 @@ private Object decode(Class cls, java.lang.reflect.Type genericType) int size = ctrlByte & 0x1f; if (size >= 29) { + // Size codes 29, 30, and 31 read one, two, and three more bytes. + this.checkDataSize(size - 28); size = switch (size) { case 29 -> 29 + (0xFF & buffer.get()); case 30 -> 285 + decodeInteger(2); @@ -187,7 +197,7 @@ private Object decode(Class cls, java.lang.reflect.Type genericType) private Object decodeTarget(CacheKey key) throws IOException { long offset = key.offset(); - if (offset >= this.buffer.capacity()) { + if (offset >= this.capacity) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); @@ -322,7 +332,7 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException throw new InvalidDatabaseException( "The MaxMind DB file's data section exceeds the maximum number of values"); } - if (valueCount > this.buffer.capacity() - this.buffer.position()) { + if (valueCount > this.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"); @@ -357,7 +367,7 @@ private void chargePayload(long length) throws InvalidDatabaseException { } private void checkDataSize(long length) throws InvalidDatabaseException { - if (length > this.buffer.capacity() - this.buffer.position()) { + if (length > this.capacity - this.buffer.position()) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " + "a value extends beyond the end of the data section."); @@ -638,6 +648,7 @@ private double decodeDouble(int size) throws InvalidDatabaseException { "The MaxMind DB file's data section contains bad data: " + "invalid size of double."); } + this.checkDataSize(8); return this.buffer.getDouble(); } @@ -647,6 +658,7 @@ private float decodeFloat(int size) throws InvalidDatabaseException { "The MaxMind DB file's data section contains bad data: " + "invalid size of float."); } + this.checkDataSize(4); return this.buffer.getFloat(); } @@ -1371,7 +1383,7 @@ private long nextValueOffset(long offset, int numberToSkip) offset += size; break; } - if (offset > this.buffer.capacity()) { + if (offset > this.capacity) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " + "a value extends beyond the end of the data section."); @@ -1382,7 +1394,7 @@ private long nextValueOffset(long offset, int numberToSkip) private CtrlData getCtrlData(long offset) throws InvalidDatabaseException { - if (offset >= this.buffer.capacity()) { + if (offset >= this.capacity) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); @@ -1395,6 +1407,7 @@ private CtrlData getCtrlData(long offset) var type = Type.fromControlByte(ctrlByte); if (type.equals(Type.EXTENDED)) { + this.checkDataSize(1); var nextByte = this.buffer.get(); var typeNum = nextByte + 7; @@ -1419,6 +1432,7 @@ private CtrlData getCtrlData(long offset) var size = ctrlByte & 0x1f; if (size >= 29) { var bytesToRead = size - 28; + this.checkDataSize(bytesToRead); offset += bytesToRead; size = switch (size) { case 29 -> 29 + (0xFF & buffer.get()); diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 855503a5..f2cbcb8b 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -16,6 +16,7 @@ import java.nio.file.Path; import java.util.ArrayList; import java.util.HashMap; +import java.util.LinkedHashMap; import java.util.List; import java.util.Map; import java.util.concurrent.TimeUnit; @@ -980,6 +981,73 @@ public void testTruncatedUnknownScalarIsRejectedAsInvalidDatabase() { assertThat(ex.getMessage(), containsString("extends beyond the end")); } + @Test + public void testTruncatedHeaderIsRejectedAsInvalidDatabase() { + var cases = new LinkedHashMap(); + // The outer array declares two elements, but the first element and its + // own child consume the rest of the buffer, so no control byte remains. + cases.put("control byte", new byte[] {0x02, 0x04, 0x01, 0x04, (byte) 0xA0}); + cases.put("extended type byte", new byte[] {0x00}); + cases.put("size code 29", new byte[] {0x5D}); + cases.put("size code 30", new byte[] {0x5E, 0x00}); + cases.put("size code 31", new byte[] {0x5F, 0x00, 0x00}); + cases.put("pointer", new byte[] {0x20}); + cases.put("double", new byte[] {0x68}); + cases.put("float", new byte[] {0x04, 0x08}); + + for (var entry : cases.entrySet()) { + for (var buffer : truncationBuffers(entry.getValue())) { + var decoder = new Decoder(NoCache.getInstance(), buffer, 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, Object.class), + entry.getKey() + ); + assertThat(entry.getKey(), ex.getMessage(), + containsString("extends beyond the end")); + } + } + } + + @Test + public void testTruncatedUnknownFieldHeaderIsRejectedAsInvalidDatabase() { + var cases = new LinkedHashMap(); + cases.put("extended type byte", new byte[] {0x00}); + cases.put("size code 29", new byte[] {0x5D}); + cases.put("size code 30", new byte[] {0x5E, 0x00}); + cases.put("size code 31", new byte[] {0x5F, 0x00, 0x00}); + + for (var entry : cases.entrySet()) { + 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(entry.getValue()); + + for (var buffer : truncationBuffers(out.toByteArray())) { + var decoder = new Decoder(NoCache.getInstance(), buffer, 0); + var ex = assertThrows( + InvalidDatabaseException.class, + () -> decoder.decode(0, EmptyModel.class), + entry.getKey() + ); + assertThat(entry.getKey(), ex.getMessage(), + containsString("extends beyond the end")); + } + } + } + + // Both buffer implementations must report a truncated read as an + // InvalidDatabaseException rather than a BufferUnderflowException or an + // IndexOutOfBoundsException. + private static List truncationBuffers(byte[] data) { + var chunks = new ByteBuffer[data.length]; + for (var i = 0; i < data.length; i++) { + chunks[i] = ByteBuffer.wrap(data, i, 1).slice(); + } + return List.of(SingleBuffer.wrap(data), new MultiBuffer(chunks, 1)); + } + @Test public void testHugeContainerIsRejectedBeforeAllocation() throws IOException { // An array control byte can declare up to ~16.8 million entries from a From 140b5a7d7b1262afd11301ac357abad38e930091 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:12:36 +0000 Subject: [PATCH 16/25] Skip unknown integer payloads without validating their widths --- src/main/java/com/maxmind/db/Decoder.java | 20 ------------------- src/test/java/com/maxmind/db/DecoderTest.java | 10 +++------- 2 files changed, 3 insertions(+), 27 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 0074de10..a0f21d32 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -1359,26 +1359,6 @@ private long nextValueOffset(long offset, int numberToSkip) break; case BOOLEAN: break; - case UINT16: - this.checkIntegerSize("uint16", size, 2); - offset += size; - break; - case UINT32: - this.checkIntegerSize("uint32", size, 4); - offset += size; - break; - case INT32: - this.checkIntegerSize("int32", size, 4); - offset += size; - break; - case UINT64: - this.checkIntegerSize("uint64", size, 8); - offset += size; - break; - case UINT128: - this.checkIntegerSize("uint128", size, 16); - offset += size; - break; default: offset += size; break; diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index f2cbcb8b..e7ec279b 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -436,7 +436,7 @@ public void testPointerBackedOversizedIntegerIsRejectedBeforePayloadRead() { } @Test - public void testSkippedOversizedIntegersAreRejected() { + public void testSkippedOversizedIntegersPreserveKnownFields() throws IOException { var invalidIntegers = Map.of( "uint16", new byte[] {(byte) 0xA3, 0, 0, 0}, "uint32", new byte[] {(byte) 0xC5, 0, 0, 0, 0, 0}, @@ -463,12 +463,8 @@ public void testSkippedOversizedIntegersAreRejected() { SingleBuffer.wrap(out.toByteArray()), 0 ); - var ex = assertThrows( - InvalidDatabaseException.class, - () -> decoder.decode(0, KnownFieldModel.class) - ); - assertThat(ex.getMessage(), containsString( - "invalid size of " + invalidInteger.getKey())); + var result = decoder.decode(0, KnownFieldModel.class); + assertEquals("ok", result.known(), invalidInteger.getKey()); } } From d87f0a5fd682373b2a9aeb75ab965ea298d5f069 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:09 +0000 Subject: [PATCH 17/25] Test packed costs against the decoder resource limits --- src/main/java/com/maxmind/db/Decoder.java | 6 ++--- src/test/java/com/maxmind/db/DecoderTest.java | 25 ++++++++++++++++--- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index a0f21d32..69b52c8f 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -49,9 +49,9 @@ class Decoder implements NodeCache.Loader { // pointer-backed maps, which use more Java frames per logical container // than inline values. A Decoder serves one decode operation on one thread, // so these fields need no synchronization. - private static final int MAX_DEPTH = 128; - private static final int MAX_VALUES = 1 << 16; - private static final long MAX_PAYLOAD_BYTES = 1 << 21; + static final int MAX_DEPTH = 128; + static final int MAX_VALUES = 1 << 16; + static final long MAX_PAYLOAD_BYTES = 1 << 21; // A collection's declared size is its logical child count, but it is not // proof that the input contains that many decodable children. When deriving diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index e7ec279b..4f6e50ab 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -29,10 +29,27 @@ public class DecoderTest { @Test public void testDecodedValueStoresMaximumCosts() { - var value = new DecodedValue(null, 1 << 16, 1L << 21, TEST_MAX_DEPTH); - assertEquals(1 << 16, value.values()); - assertEquals(1L << 21, value.payloadBytes()); - assertEquals(TEST_MAX_DEPTH, value.depth()); + var value = new DecodedValue(null, Decoder.MAX_VALUES, Decoder.MAX_PAYLOAD_BYTES, Decoder.MAX_DEPTH); + assertEquals(Decoder.MAX_VALUES, value.values()); + assertEquals(Decoder.MAX_PAYLOAD_BYTES, value.payloadBytes()); + assertEquals(Decoder.MAX_DEPTH, value.depth()); + } + + @Test + public void testDecodedValueCostsAreIndependent() { + var costs = new long[][] { + {0, 0, 0}, + {Decoder.MAX_VALUES, 0, 0}, + {0, Decoder.MAX_PAYLOAD_BYTES, 0}, + {0, 0, Decoder.MAX_DEPTH}, + {123, 456, 7}, + }; + for (var expected : costs) { + var value = new DecodedValue(null, (int) expected[0], expected[1], (int) expected[2]); + assertEquals(expected[0], value.values(), "value cost"); + assertEquals(expected[1], value.payloadBytes(), "payload cost"); + assertEquals(expected[2], value.depth(), "depth cost"); + } } private static Map int32() { From 791a050b058e35189f5b0913c6a01d01abb84b03 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:24 +0000 Subject: [PATCH 18/25] Run pointer fan-out tests in a bounded subprocess --- src/test/java/com/maxmind/db/DecoderTest.java | 2 +- src/test/java/com/maxmind/db/ReaderTest.java | 29 ++++++++++++++----- 2 files changed, 23 insertions(+), 8 deletions(-) diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 4f6e50ab..f936cdc5 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -785,7 +785,7 @@ public void testContainerDepthFitsReducedThreadStack() throws Exception { runProbe("-Xss512k", StackProbe.class); } - private static void runProbe(String vmArgument, Class probe) throws Exception { + static void runProbe(String vmArgument, Class probe) throws Exception { var executable = System.getProperty("os.name").startsWith("Windows") ? "java.exe" : "java"; diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index c998d28c..c5385efa 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2203,9 +2203,26 @@ public void testNullToPrimitiveErrorMessage(int chunkSize) throws IOException { } } - @ParameterizedTest - @MethodSource("chunkSizes") - public void testPointerFanOutIsRejected(int chunkSize) throws IOException { + @Test + public void testPointerFanOutIsRejected() throws Exception { + DecoderTest.runProbe("-Xmx128m", FanOutProbe.class); + } + + public static final class FanOutProbe { + private FanOutProbe() { + } + + public static void main(String[] args) throws IOException { + var tests = new ReaderTest(); + for (var chunkSize : chunkSizes().toArray()) { + tests.checkPointerFanOutIsRejected(chunkSize); + } + tests.checkPointerFanOutIsRejectedForMemoryAndStreamReaders(); + tests.checkPointerFanOutIsRejectedWithCachedTargets(); + } + } + + private void checkPointerFanOutIsRejected(int chunkSize) throws IOException { var fixtures = new String[] { "MaxMind-DB-test-pointer-decoder-dos.mmdb", "MaxMind-DB-test-pointer-decoder-dos-ipv6.mmdb", @@ -2224,8 +2241,7 @@ public void testPointerFanOutIsRejected(int chunkSize) throws IOException { } } - @Test - public void testPointerFanOutIsRejectedForMemoryAndStreamReaders() throws IOException { + private void checkPointerFanOutIsRejectedForMemoryAndStreamReaders() throws IOException { var fixture = "MaxMind-DB-test-pointer-decoder-dos.mmdb"; var address = InetAddress.getByName("1.1.1.1"); try (var memoryReader = new Reader(getFile(fixture), FileMode.MEMORY, 512)) { @@ -2240,8 +2256,7 @@ public void testPointerFanOutIsRejectedForMemoryAndStreamReaders() throws IOExce } } - @Test - public void testPointerFanOutIsRejectedWithCachedTargets() throws IOException { + private void checkPointerFanOutIsRejectedWithCachedTargets() throws IOException { var fixture = "MaxMind-DB-test-pointer-decoder-dos.mmdb"; try (var reader = new Reader(getFile(fixture), new CHMCache())) { var address = InetAddress.getByName("1.1.1.1"); From f314068b689f70694591b13e2fbf4d505e462dd1 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:24 +0000 Subject: [PATCH 19/25] Cover payload and skipped-depth boundaries across cache states --- src/test/java/com/maxmind/db/DecoderTest.java | 29 ++++++----- src/test/java/com/maxmind/db/ReaderTest.java | 50 +++++++++++++++---- 2 files changed, 56 insertions(+), 23 deletions(-) diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index f936cdc5..815aed69 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -902,20 +902,23 @@ public void testUnknownFieldValueCountIsBounded() { } @Test - public void testUnknownFieldDepthIsBounded() { - var value = nestedArrays(TEST_MAX_DEPTH); - 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(), + public void testUnknownFieldDepthIsBounded() throws IOException { + for (var depth : new int[] {TEST_MAX_DEPTH - 1, TEST_MAX_DEPTH}) { + var out = new ByteArrayOutputStream(); + out.write(0xE1); // map with one key/value pair + out.write(0x47); + out.writeBytes("unknown".getBytes(StandardCharsets.UTF_8)); + out.writeBytes(nestedArrays(depth)); + 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")); + if (depth == TEST_MAX_DEPTH - 1) { + assertInstanceOf(EmptyModel.class, decoder.decode(0, EmptyModel.class)); + } else { + var ex = assertThrows(InvalidDatabaseException.class, + () -> decoder.decode(0, EmptyModel.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum depth")); + } + } } @Test diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index c5385efa..dcf065be 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2312,14 +2312,13 @@ public void testPayloadAmplificationIsRejected() throws IOException { for (var cache : List.of( NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { try (var reader = new Reader(getFile(fixture), cache)) { - var ex = assertThrows( - InvalidDatabaseException.class, - () -> reader.get(ip, Object.class), - fixture + " should be rejected"); - assertThat( - ex.getMessage(), - containsString("exceeds the maximum payload size") - ); + for (var attempt = 0; attempt < 2; attempt++) { + var ex = assertThrows( + InvalidDatabaseException.class, + () -> reader.get(ip, Object.class), + fixture + ", " + cache.getClass().getSimpleName() + ", attempt " + attempt); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } } } } @@ -2333,8 +2332,39 @@ public void testPayloadAtLimitDecodes() throws IOException { NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { try (var reader = new Reader( getFile("MaxMind-DB-test-decoder-payload-limit.mmdb"), cache)) { - var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); - assertNotNull(value); + for (var attempt = 0; attempt < 2; attempt++) { + var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); + assertPayloadAtLimit(value); + } + } + } + } + + private static void assertPayloadAtLimit(Object value) { + var values = (List) value; + assertEquals(33, values.size()); + var large = new byte[65_535]; + for (var i = 0; i < 32; i++) { + assertArrayEquals(large, (byte[]) values.get(i), "payload " + i); + } + assertArrayEquals(new byte[32], (byte[]) values.get(32), "final payload"); + } + + @Test + public void testValueLimitFixturesUseJavaAccounting() throws IOException { + // These fixtures use flat specification counts. Java also charges each pointer. + for (var suffix : List.of("value-limit", "value-limit-over", "value-limit-pointer-heavy")) { + var fixture = "MaxMind-DB-test-decoder-" + suffix + ".mmdb"; + for (var cache : List.of( + NoCache.getInstance(), new CHMCache(), new CHMCache(0))) { + try (var reader = new Reader(getFile(fixture), cache)) { + for (var attempt = 0; attempt < 2; attempt++) { + var ex = assertThrows(InvalidDatabaseException.class, + () -> reader.get(InetAddress.getByName("1.1.1.1"), Object.class), + fixture + ", " + cache.getClass().getSimpleName() + ", attempt " + attempt); + assertThat(ex.getMessage(), containsString("exceeds the maximum number of values")); + } + } } } } From 553be73ec8d541c6f014a2d4b1a2e83a76e019c1 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:24 +0000 Subject: [PATCH 20/25] Test payload boundaries with concurrent shared-cache lookups --- src/test/java/com/maxmind/db/ReaderTest.java | 42 ++++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index dcf065be..881047e6 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -28,7 +28,11 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.concurrent.Callable; import java.util.concurrent.ConcurrentHashMap; +import java.util.concurrent.CyclicBarrier; +import java.util.concurrent.Executors; +import java.util.concurrent.TimeUnit; import java.util.stream.IntStream; import org.junit.jupiter.api.AfterEach; @@ -2369,6 +2373,44 @@ public void testValueLimitFixturesUseJavaAccounting() throws IOException { } } + @Test + public void testSharedCachePreservesPayloadBoundaries() throws Exception { + var executor = Executors.newFixedThreadPool(4); + try { + for (var overLimit : new boolean[] {false, true}) { + var fixture = "MaxMind-DB-test-decoder-payload-limit"; + if (overLimit) { + fixture += "-over"; + } + try (var reader = new Reader(getFile(fixture + ".mmdb"), new CHMCache())) { + var start = new CyclicBarrier(4); + var tasks = new ArrayList>(); + for (var worker = 0; worker < 4; worker++) { + tasks.add(() -> { + start.await(15, TimeUnit.SECONDS); + for (var attempt = 0; attempt < 2; attempt++) { + if (overLimit) { + var ex = assertThrows(InvalidDatabaseException.class, + () -> reader.get(InetAddress.getByName("1.1.1.1"), Object.class)); + assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); + } else { + assertPayloadAtLimit(reader.get(InetAddress.getByName("1.1.1.1"), Object.class)); + } + } + return null; + }); + } + for (var future : executor.invokeAll(tasks, 15, TimeUnit.SECONDS)) { + future.get(); + } + } + } + } finally { + executor.shutdownNow(); + assertTrue(executor.awaitTermination(15, TimeUnit.SECONDS), "cache workers did not stop"); + } + } + // 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 From a0ed65ef4bfb89bcd8a9429403271b44f082f27b Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:44 +0000 Subject: [PATCH 21/25] Remove redundant decoder cost accessors and size casts --- src/main/java/com/maxmind/db/DecodedValue.java | 12 ------------ src/main/java/com/maxmind/db/Decoder.java | 4 ++-- src/test/java/com/maxmind/db/DecoderTest.java | 14 +++++++------- 3 files changed, 9 insertions(+), 21 deletions(-) diff --git a/src/main/java/com/maxmind/db/DecodedValue.java b/src/main/java/com/maxmind/db/DecodedValue.java index a3f37eb1..85c905ef 100644 --- a/src/main/java/com/maxmind/db/DecodedValue.java +++ b/src/main/java/com/maxmind/db/DecodedValue.java @@ -25,26 +25,14 @@ Object value() { return value; } - int values() { - return values(costs()); - } - static int values(long costs) { return (int) (costs >>> VALUES_SHIFT); } - long payloadBytes() { - return payloadBytes(costs()); - } - static long payloadBytes(long costs) { return (costs >>> PAYLOAD_SHIFT) & PAYLOAD_MASK; } - int depth() { - return depth(costs()); - } - static int depth(long costs) { return (int) (costs & 0xFF); } diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 69b52c8f..a8bb0393 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -575,13 +575,13 @@ private static Object coerceFromBigInteger(BigInteger value, Class target) { return value; } - private String decodeString(long size) throws IOException { + private String decodeString(int size) throws IOException { this.chargePayload(size); // Performance optimization: String's UTF-8 path avoids the temporary // CharBuffer and char[] used by CharsetDecoder, despite this byte[] copy. // On OpenJDK 26, random GeoLite2-City lookup throughput improved by about // 6% with CHMCache and 22% without caching over the previous decoder. - var bytes = new byte[(int) size]; + var bytes = new byte[size]; this.buffer.get(bytes); var value = new String(bytes, UTF_8); // String replaces malformed UTF-8 with U+FFFD. Validate strings containing diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index 815aed69..f1295245 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -30,9 +30,9 @@ public class DecoderTest { @Test public void testDecodedValueStoresMaximumCosts() { var value = new DecodedValue(null, Decoder.MAX_VALUES, Decoder.MAX_PAYLOAD_BYTES, Decoder.MAX_DEPTH); - assertEquals(Decoder.MAX_VALUES, value.values()); - assertEquals(Decoder.MAX_PAYLOAD_BYTES, value.payloadBytes()); - assertEquals(Decoder.MAX_DEPTH, value.depth()); + assertEquals(Decoder.MAX_VALUES, DecodedValue.values(value.costs())); + assertEquals(Decoder.MAX_PAYLOAD_BYTES, DecodedValue.payloadBytes(value.costs())); + assertEquals(Decoder.MAX_DEPTH, DecodedValue.depth(value.costs())); } @Test @@ -46,9 +46,9 @@ public void testDecodedValueCostsAreIndependent() { }; for (var expected : costs) { var value = new DecodedValue(null, (int) expected[0], expected[1], (int) expected[2]); - assertEquals(expected[0], value.values(), "value cost"); - assertEquals(expected[1], value.payloadBytes(), "payload cost"); - assertEquals(expected[2], value.depth(), "depth cost"); + assertEquals(expected[0], DecodedValue.values(value.costs()), "value cost"); + assertEquals(expected[1], DecodedValue.payloadBytes(value.costs()), "payload cost"); + assertEquals(expected[2], DecodedValue.depth(value.costs()), "depth cost"); } } @@ -1200,7 +1200,7 @@ public void testTruncatedPayloadsAreRejectedAsInvalidDatabase() { } @Test - public void testInvalidStringDoesNotChangeBufferLimit() throws IOException { + public void testDecoderCanBeReusedAfterInvalidString() throws IOException { var data = new byte[] {0x41, (byte) 0xFF, 0x41, 'a'}; var decoder = new Decoder(NoCache.getInstance(), SingleBuffer.wrap(data), 0); From 073143089512a0772886c1fd201036d0247044b4 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:13:44 +0000 Subject: [PATCH 22/25] Describe file bounds accurately in decoder errors --- src/main/java/com/maxmind/db/Decoder.java | 6 +++--- src/test/java/com/maxmind/db/DecoderTest.java | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index a8bb0393..9fd93ba4 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -335,7 +335,7 @@ private void checkContainerSize(long valueCount) throws InvalidDatabaseException if (valueCount > this.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"); + + "a container declares more entries than the database can hold"); } } @@ -370,7 +370,7 @@ private void checkDataSize(long length) throws InvalidDatabaseException { if (length > this.capacity - this.buffer.position()) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " - + "a value extends beyond the end of the data section."); + + "a value extends beyond the end of the database."); } } @@ -1366,7 +1366,7 @@ private long nextValueOffset(long offset, int numberToSkip) if (offset > this.capacity) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains bad data: " - + "a value extends beyond the end of the data section."); + + "a value extends beyond the end of the database."); } } return offset; diff --git a/src/test/java/com/maxmind/db/DecoderTest.java b/src/test/java/com/maxmind/db/DecoderTest.java index f1295245..fb2746e3 100644 --- a/src/test/java/com/maxmind/db/DecoderTest.java +++ b/src/test/java/com/maxmind/db/DecoderTest.java @@ -1126,7 +1126,7 @@ public void testImpossibleArrayIsRejectedBeforeAllocation() { InvalidDatabaseException.class, () -> decoder.decode(0, Object.class)); assertThat(ex.getMessage(), containsString( - "a container declares more entries than the data section can hold")); + "a container declares more entries than the database can hold")); } @Test @@ -1139,7 +1139,7 @@ public void testImpossibleMapIsRejectedBeforeAllocation() { InvalidDatabaseException.class, () -> decoder.decode(0, Object.class)); assertThat(ex.getMessage(), containsString( - "a container declares more entries than the data section can hold")); + "a container declares more entries than the database can hold")); } @Test From 715e9994c1720708c3cb7bd73aa1387d638affa1 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:16:19 +0000 Subject: [PATCH 23/25] Identify payload boundary assertions by cache and attempt --- src/test/java/com/maxmind/db/ReaderTest.java | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/test/java/com/maxmind/db/ReaderTest.java b/src/test/java/com/maxmind/db/ReaderTest.java index 881047e6..1b664cda 100644 --- a/src/test/java/com/maxmind/db/ReaderTest.java +++ b/src/test/java/com/maxmind/db/ReaderTest.java @@ -2338,20 +2338,20 @@ public void testPayloadAtLimitDecodes() throws IOException { getFile("MaxMind-DB-test-decoder-payload-limit.mmdb"), cache)) { for (var attempt = 0; attempt < 2; attempt++) { var value = reader.get(InetAddress.getByName("1.1.1.1"), Object.class); - assertPayloadAtLimit(value); + assertPayloadAtLimit(value, cache.getClass().getSimpleName() + ", attempt " + attempt); } } } } - private static void assertPayloadAtLimit(Object value) { + private static void assertPayloadAtLimit(Object value, String context) { var values = (List) value; - assertEquals(33, values.size()); + assertEquals(33, values.size(), context); var large = new byte[65_535]; for (var i = 0; i < 32; i++) { - assertArrayEquals(large, (byte[]) values.get(i), "payload " + i); + assertArrayEquals(large, (byte[]) values.get(i), context + ", payload " + i); } - assertArrayEquals(new byte[32], (byte[]) values.get(32), "final payload"); + assertArrayEquals(new byte[32], (byte[]) values.get(32), context + ", final payload"); } @Test @@ -2394,7 +2394,8 @@ public void testSharedCachePreservesPayloadBoundaries() throws Exception { () -> reader.get(InetAddress.getByName("1.1.1.1"), Object.class)); assertThat(ex.getMessage(), containsString("exceeds the maximum payload size")); } else { - assertPayloadAtLimit(reader.get(InetAddress.getByName("1.1.1.1"), Object.class)); + assertPayloadAtLimit(reader.get(InetAddress.getByName("1.1.1.1"), Object.class), + "shared cache, attempt " + attempt); } } return null; From e0fa7852c1747d9e7c16eb277d9c3f86bbda5828 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:19:10 +0000 Subject: [PATCH 24/25] Clarify decoder accounting and cache contracts --- .../java/com/maxmind/db/DecodedValue.java | 7 +-- src/main/java/com/maxmind/db/Decoder.java | 57 ++++++------------- src/main/java/com/maxmind/db/NodeCache.java | 4 ++ 3 files changed, 24 insertions(+), 44 deletions(-) diff --git a/src/main/java/com/maxmind/db/DecodedValue.java b/src/main/java/com/maxmind/db/DecodedValue.java index 85c905ef..8baf7145 100644 --- a/src/main/java/com/maxmind/db/DecodedValue.java +++ b/src/main/java/com/maxmind/db/DecodedValue.java @@ -1,17 +1,16 @@ package com.maxmind.db; /** - * {@code DecodedValue} is a wrapper for the decoded value. + * An opaque decoded value and its resource costs, produced by {@link NodeCache.Loader}. + * Caches retain this instance unchanged for its original key. See {@link NodeCache}. */ public final class DecodedValue { private static final int PAYLOAD_SHIFT = 8; private static final int VALUES_SHIFT = 30; private static final long PAYLOAD_MASK = (1L << 22) - 1; + // Final fields preserve their initialized values when a cache publishes this object. final Object value; - // A NodeCache is user-supplied and may publish this instance to another - // thread without a happens-before edge. Set the costs in the constructor - // so a reader cannot see a zero budget charge for a non-empty value. private final long costs; DecodedValue(Object value, int values, long payloadBytes, int depth) { diff --git a/src/main/java/com/maxmind/db/Decoder.java b/src/main/java/com/maxmind/db/Decoder.java index 9fd93ba4..c1b57f7c 100644 --- a/src/main/java/com/maxmind/db/Decoder.java +++ b/src/main/java/com/maxmind/db/Decoder.java @@ -35,30 +35,20 @@ class Decoder implements NodeCache.Loader { private final NodeCache cache; - // Per-operation resource limits. The MaxMind DB specification recommends - // depth and value limits, but permits equivalent reader-specific accounting. - // This decoder charges each decoded or skipped value. Each pointer occurrence - // also consumes the logical cost of its target. Cache misses measure that cost, - // and cache hits replay it. This keeps accounting independent of cache state - // rather than following the specification's example flat value count. - // Container depth, together with rejecting illegal pointer-to-pointer values, - // bounds recursive calls. - // The payload limit bounds encoded string and bytes data materialized by - // this Java decoder. - // The lower depth limit leaves room on a 512 KiB thread stack even for - // pointer-backed maps, which use more Java frames per logical container - // than inline values. A Decoder serves one decode operation on one thread, - // so these fields need no synchronization. + // Bound work per operation. Decoded pointers cost one value plus their target. + // Skipped pointers cost one and are not followed. Depth counts containers only. + // Rejecting followed pointer-to-pointer values bounds data-driven recursion. + // The 128-container limit is tested on a 512 KiB stack. Payload accounting + // covers materialized string and bytes data. static final int MAX_DEPTH = 128; static final int MAX_VALUES = 1 << 16; static final long MAX_PAYLOAD_BYTES = 1 << 21; - // A collection's declared size is its logical child count, but it is not - // proof that the input contains that many decodable children. When deriving - // an initial capacity from it, limit unused capacity on the active recursion - // path. Completed children remain bounded by MAX_VALUES. + // Nested containers can each declare nearly MAX_VALUES children before any + // are decoded. Cap preallocation to avoid reserving unused slots at every depth. private static final int MAX_INITIAL_COLLECTION_CAPACITY = 128; private int depth; + // Maximum absolute depth while measuring a cached target, or -1 outside load(). private int maxDepth = -1; private int valuesRemaining = MAX_VALUES; private long payloadRemaining = MAX_PAYLOAD_BYTES; @@ -202,9 +192,8 @@ private Object decodeTarget(CacheKey key) throws IOException { "The MaxMind DB file's data section contains bad data: " + "pointer larger than the database."); } - // Validate a target when the cache loader decodes it. A target that was - // loaded successfully has already passed this check, so cache hits do - // not need to reread its control byte. + // Validate each followed target before decoding. Cached targets have + // already passed this check. if (Type.fromControlByte(0xFF & this.buffer.get(offset)) == Type.POINTER) { throw new InvalidDatabaseException( "The MaxMind DB file's data section contains a pointer to a pointer"); @@ -217,6 +206,8 @@ private Object decodeTarget(CacheKey key) throws IOException { @Override public DecodedValue load(CacheKey key) throws IOException { + // Measure within the caller's budget and absolute depth. Restore the + // counters here so charge() applies the measured cost exactly once. var valuesRemaining = this.valuesRemaining; var payloadRemaining = this.payloadRemaining; var depth = this.depth; @@ -318,16 +309,9 @@ private static boolean isSimpleType(Class cls) { || cls == 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). + // Check the remaining value budget and file bytes before allocating. + // valueCount counts array elements, or both keys and values for maps. private void checkContainerSize(long valueCount) throws InvalidDatabaseException { - // A container cannot decode more values than the per-operation 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"); @@ -351,12 +335,7 @@ private void enterContainer(long valueCount) throws InvalidDatabaseException { } } - // Charge a string or bytes payload against the per-operation budget before - // materializing it. A payload amplification points many pointers at one large - // value. Cached targets retain their logical payload cost, so each pointer - // occurrence consumes the cost even when the decoder reuses the value. The - // comparison is against the remaining budget so it cannot overflow. The - // limit is inclusive: a total exactly at the limit is allowed. + // Check payload bounds before allocation. A total exactly at the limit is allowed. private void chargePayload(long length) throws InvalidDatabaseException { if (length > this.payloadRemaining) { throw new InvalidDatabaseException( @@ -577,10 +556,8 @@ private static Object coerceFromBigInteger(BigInteger value, Class target) { private String decodeString(int size) throws IOException { this.chargePayload(size); - // Performance optimization: String's UTF-8 path avoids the temporary - // CharBuffer and char[] used by CharsetDecoder, despite this byte[] copy. - // On OpenJDK 26, random GeoLite2-City lookup throughput improved by about - // 6% with CHMCache and 22% without caching over the previous decoder. + // String's UTF-8 path avoids the temporary CharBuffer and char[] used by + // CharsetDecoder, despite this byte[] copy. var bytes = new byte[size]; this.buffer.get(bytes); var value = new String(bytes, UTF_8); diff --git a/src/main/java/com/maxmind/db/NodeCache.java b/src/main/java/com/maxmind/db/NodeCache.java index 4dd9175c..c934b48f 100644 --- a/src/main/java/com/maxmind/db/NodeCache.java +++ b/src/main/java/com/maxmind/db/NodeCache.java @@ -5,6 +5,10 @@ /** * NodeCache is an interface for a cache that stores decoded values from the * data section of the database. + * + *

Scope each cache to one {@link Reader}. Return the loader's result unchanged + * for the requested key, propagate loader exceptions, and never return {@code null}. + * Implementations used for concurrent lookups must be thread-safe. */ public interface NodeCache { /** From a32621914356ff53b77c75a794a2ed1b6bad6906 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Tue, 8 Sep 2026 22:19:10 +0000 Subject: [PATCH 25/25] Consolidate release notes and document skipped-field behavior --- CHANGELOG.md | 25 +++++++------------------ UPGRADING.md | 21 ++++++++++----------- 2 files changed, 17 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 325dc178..5a8d30df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,28 +4,17 @@ CHANGELOG 4.2.0 ------------------ -* Fixed decoding of data pointers with offsets of 2 GiB or greater. The - pointer payload was decoded into an `int`, so such offsets were - sign-extended to a negative value and rejected by `Buffer.position()` - 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 decoding of data pointers with offsets of 2 GiB or greater. Records + beyond that boundary could previously fail with `IllegalArgumentException`. * Fixed skipping unknown four-byte pointers during typed decoding. Skipped - values that extend past the data section are now rejected. MaxMind-produced - databases were unaffected. -* Fixed UTF-8 decoding across buffer chunks and rejection of incomplete - multibyte characters at the end of a string. -* Fixed the exception thrown for a database truncated in the middle of a - value. Reading a control byte, an extended type byte, a size header, a - pointer, a `double`, or a `float` past the end of the data section threw - `BufferUnderflowException` or `IndexOutOfBoundsException`. These now throw - `InvalidDatabaseException`, as the rest of the reader does. + values that extend past the database are now rejected. +* Fixed UTF-8 decoding across buffer chunks. Malformed decoded strings and + truncated values now throw `InvalidDatabaseException`. * Added decoder limits to prevent excessive CPU and memory use from crafted databases: 65,536 decoded or skipped values, 128 nested containers, and 2 MiB of encoded string and bytes payload per operation. Exceeding a limit throws - `InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for details. - * Oversized integer encodings are rejected. - * Truncated payloads and malformed UTF-8 are rejected. + `InvalidDatabaseException`. See [UPGRADING.md](UPGRADING.md) for accounting, + decoded-value validation, and collection capacity-hint changes. * Improved decoder performance and reduced per-lookup allocation, including UTF-8 string decoding. diff --git a/UPGRADING.md b/UPGRADING.md index 2209552d..e8b6b5fb 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -9,24 +9,23 @@ The decoder rejects an operation that exceeds any of these limits: - 128 nested maps or arrays - 2 MiB of encoded string and bytes payload materialized by the decoder -Each cached pointer target retains its logical value, depth, and payload cost. -Every decoded pointer occurrence consumes that recorded cost. The cache still -avoids decoding or materializing the target again, but cache state does not -determine whether an operation exceeds a limit. A pointer in a field the -decoder skips counts as one value. The decoder does not visit its target. +A decoded pointer costs one value in addition to its target's logical costs. +Cached targets retain their value count, container depth, and payload bytes, so +cache state does not change whether a decode exceeds a limit. Skipped pointers +count as one value and their targets remain unvisited. Skipped fields receive structural bounds and +resource checks, but their contents are not fully validated. These limits leave a wide margin above MaxMind-produced records. A custom database containing an unusually large record that decoded in an earlier release may now throw `InvalidDatabaseException`. The limits are not configurable in this release. -When the decoder constructs a custom `List` or `Map` type through an `int` -constructor, it passes an initial-capacity hint capped at 128 rather than the -full declared collection size. +Initial collection capacity hints are capped at 128. Built-in collections grow +as needed. Custom `List` and `Map` types constructed through an `int` constructor +receive this capped hint instead of the full declared size. -The decoder also rejects a data-section pointer whose target is another pointer, -which the MaxMind DB format does not permit. It rejects integer payloads wider -than their format type permits before reading the payload. +When following a pointer, the decoder rejects targets that are themselves +pointers. Decoded integers wider than their format type permits are also rejected. # Upgrading to 4.0.0