diff --git a/bson/src/main/org/bson/BsonBinaryReader.java b/bson/src/main/org/bson/BsonBinaryReader.java index 5fff43beef..06d2d70656 100644 --- a/bson/src/main/org/bson/BsonBinaryReader.java +++ b/bson/src/main/org/bson/BsonBinaryReader.java @@ -304,10 +304,10 @@ protected void doSkipValue() { int skip; switch (getCurrentBsonType()) { case ARRAY: - skip = readSize() - 4; + skip = readContainerSize(5) - 4; break; case BINARY: - skip = readSize() + 1; + skip = addSkipBytes(readSize(), 1); break; case BOOLEAN: skip = 1; @@ -316,7 +316,7 @@ protected void doSkipValue() { skip = 8; break; case DOCUMENT: - skip = readSize() - 4; + skip = readContainerSize(5) - 4; break; case DOUBLE: skip = 8; @@ -331,10 +331,10 @@ protected void doSkipValue() { skip = 16; break; case JAVASCRIPT: - skip = readSize(); + skip = readStringSize(); break; case JAVASCRIPT_WITH_SCOPE: - skip = readSize() - 4; + skip = readJavaScriptWithScopeSize(); break; case MAX_KEY: skip = 0; @@ -354,10 +354,10 @@ protected void doSkipValue() { skip = 0; break; case STRING: - skip = readSize(); + skip = readStringSize(); break; case SYMBOL: - skip = readSize(); + skip = readStringSize(); break; case TIMESTAMP: skip = 8; @@ -366,7 +366,7 @@ protected void doSkipValue() { skip = 0; break; case DB_POINTER: - skip = readSize() + 12; // String followed by ObjectId + skip = addSkipBytes(readStringSize(), 12); // String followed by ObjectId break; default: throw new BSONException("Unexpected BSON type: " + getCurrentBsonType()); @@ -385,6 +385,53 @@ private int readSize() { return size; } + private int readStringSize() { + int size = readSize(); + if (size == 0) { + throw new BsonSerializationException( + "While reading a BSON string found a size of 0; expected at least 1 for the null terminator."); + } + return size; + } + + private int readContainerSize(final int minSize) { + int size = readSize(); + if (size < minSize) { + throw new BsonSerializationException(format( + "While reading a BSON container found a size of %d, which is less than the minimum of %d.", + size, minSize)); + } + return size; + } + + private int readJavaScriptWithScopeSize() { + // int32 totalSize + minimal string (int32 size + '\x00') + minimal document (int32 size + '\x00') = 14 + int size = readContainerSize(14); + int codeSize = readStringSize(); + // 13 = 4 codeSize prefix + 4 scopeSize prefix + 5 minimum scope document + if (codeSize > size - 13) { + throw new BsonSerializationException(format( + "While reading a BSON javascript_with_scope found code size %d is too large for total size %d.", + codeSize, size)); + } + bsonInput.skip(codeSize); + int scopeSize = readContainerSize(5); + long actualSize = 4L + 4 + codeSize + scopeSize; + if (actualSize != size) { + throw new BsonSerializationException(format("Expected size to be %d, not %d.", size, actualSize)); + } + return scopeSize - 4; + } + + private int addSkipBytes(final int size, final int trailingBytes) { + if (size > Integer.MAX_VALUE - trailingBytes) { + throw new BsonSerializationException(format( + "While skipping a BSON value found a size of %d, which is too large to skip %d trailing bytes.", + size, trailingBytes)); + } + return size + trailingBytes; + } + protected Context getContext() { return (Context) super.getContext(); } diff --git a/bson/src/test/unit/org/bson/BsonBinaryReaderTest.java b/bson/src/test/unit/org/bson/BsonBinaryReaderTest.java index bffda74eca..dd82266093 100644 --- a/bson/src/test/unit/org/bson/BsonBinaryReaderTest.java +++ b/bson/src/test/unit/org/bson/BsonBinaryReaderTest.java @@ -25,6 +25,7 @@ import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; import static org.junit.jupiter.api.Assertions.fail; public class BsonBinaryReaderTest { @@ -70,7 +71,302 @@ public void testInvalidBsonTypeFollowedByInvalidCString() { } } - private BsonBinaryReader createReaderForBytes(final byte[] bytes) { + // String rejection tests - skipValue() rejects size=0 + + @Test + public void testSkipValueRejectsZeroLengthString() { + // {"a": } + // doc size=12: 4(size) + 1(type 0x02) + 2(name "a\0") + 4(string size=0) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.STRING), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // string size: 0 (invalid) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsZeroLengthSymbol() { + // {"a": } + // doc size=12: 4(size) + 1(type 0x0E) + 2(name "a\0") + 4(symbol size=0) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.SYMBOL), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // symbol size: 0 (invalid) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsZeroLengthJavaScript() { + // {"a": } + // doc size=12: 4(size) + 1(type 0x0D) + 2(name "a\0") + 4(js size=0) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // javascript size: 0 (invalid) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsZeroLengthDbPointer() { + // {"a": } + // doc size=24: 4(size) + 1(type 0x0C) + 2(name "a\0") + 4(string size=0) + 12(objectid) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 24, 0, 0, 0, // document size + typeByte(BsonType.DB_POINTER), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // string size: 0 (invalid) + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // objectid (12 bytes) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsOverflowingBinarySize() { + // {"a": } + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.BINARY), + 0x61, 0x00, // name: "a\0" + -1, -1, -1, 0x7F, // binary size: Integer.MAX_VALUE + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsOverflowingDbPointerSize() { + // {"a": } + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.DB_POINTER), + 0x61, 0x00, // name: "a\0" + -1, -1, -1, 0x7F, // string size: Integer.MAX_VALUE + 0x00 // document terminator + }); + } + + // Container rejection tests - skipValue() rejects undersized containers + + @Test + public void testSkipValueRejectsUndersizedDocument() { + // {"a": } + // doc size=12: 4(size) + 1(type 0x03) + 2(name "a\0") + 4(embedded doc size=3) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.DOCUMENT), + 0x61, 0x00, // name: "a\0" + 3, 0, 0, 0, // embedded document size: 3 (invalid, minimum is 5) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsUndersizedArray() { + // {"a": } + // doc size=12: 4(size) + 1(type 0x04) + 2(name "a\0") + 4(array size=0) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 12, 0, 0, 0, // document size + typeByte(BsonType.ARRAY), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // array size: 0 (invalid, minimum is 5) + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsUndersizedJavaScriptWithScope() { + // {"a": } + // doc size=24: 4(size) + 1(type 0x0F) + 2(name "a\0") + 4(js_ws size=13) + 12(filler) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 24, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 13, 0, 0, 0, // javascript_with_scope size: 13 (invalid, minimum is 14) + 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // filler + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsJavaScriptWithScopeWithZeroLengthCode() { + // {"a": } + // doc size=22: 4(size) + 1(type 0x0F) + 2(name "a\0") + 14(js_ws value) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 22, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 14, 0, 0, 0, // javascript_with_scope total size + 0, 0, 0, 0, // code string size: 0 (invalid) + 0, 0, 0, 0, 0, 0, // filler + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsJavaScriptWithScopeWithOversizedCode() { + // {"a": } + // totalSize=15, codeSize=3: codeSize (3) > totalSize - 13 (2), so the intermediate guard fires. + // doc size=23: 4(size) + 1(type 0x0F) + 2(name "a\0") + 15(js_ws value) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 23, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 15, 0, 0, 0, // javascript_with_scope total size: 15 + 3, 0, 0, 0, // code string size: 3 (invalid - too large; max is totalSize-13=2) + 0, 0, 0, 0, 0, 0, 0, 0, // filler + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueRejectsJavaScriptWithScopeWithMismatchedTotalSize() { + // {"a": } + // totalSize=20, codeSize=1, scopeSize=5: parts sum to 4+4+1+5=14, which != totalSize 20. + // Exercises the final expectedSize != size consistency check. + // doc size=28: 4(size) + 1(type 0x0F) + 2(name "a\0") + 20(js_ws value) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 28, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 20, 0, 0, 0, // javascript_with_scope total size: 20 (inconsistent with parts) + 1, 0, 0, 0, // code string size: 1 + 0x00, // code string content: "\0" + 5, 0, 0, 0, // scope document size: 5 (valid minimum) + 0x00, // scope document terminator + 0, 0, 0, 0, 0, 0, // filler so outer doc reaches declared total + 0x00 // outer document terminator + }); + } + + @Test + public void testSkipValueRejectsJavaScriptWithScopeWithUndersizedScopeDocument() { + // {"a": } + // doc size=22: 4(size) + 1(type 0x0F) + 2(name "a\0") + 14(js_ws value) + 1(doc term) + assertSkipValueThrows(new byte[]{ + 22, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 14, 0, 0, 0, // javascript_with_scope total size + 1, 0, 0, 0, // code string size: 1 + 0x00, // code string content: "\0" + 4, 0, 0, 0, // scope document size: 4 (invalid, minimum is 5) + 0x00, // filler + 0x00 // document terminator + }); + } + + // Regression guards - valid minimal values still work + + @Test + public void testSkipValueAcceptsMinimalString() { + // {"a": ""} - string with size=1 (just the null terminator) + // doc size=13: 4(size) + 1(type 0x02) + 2(name "a\0") + 4(string size=1) + 1('\0') + 1(doc term) + assertSkipValueSucceeds(new byte[]{ + 13, 0, 0, 0, // document size + typeByte(BsonType.STRING), + 0x61, 0x00, // name: "a\0" + 1, 0, 0, 0, // string size: 1 (valid minimum - just null terminator) + 0x00, // string content: "\0" + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueAcceptsEmptyBinary() { + // {"a": } + // doc size=13: 4(size) + 1(type 0x05) + 2(name "a\0") + 4(binary size=0) + 1(subtype) + 1(doc term) + assertSkipValueSucceeds(new byte[]{ + 13, 0, 0, 0, // document size + typeByte(BsonType.BINARY), + 0x61, 0x00, // name: "a\0" + 0, 0, 0, 0, // binary size: 0 (valid - empty binary) + 0x00, // subtype: generic + 0x00 // document terminator + }); + } + + @Test + public void testSkipValueAcceptsMinimalDocument() { + // {"a": {}} - embedded document with size=5 (minimum valid: int32 size + '\x00') + // doc size=13: 4(docsize) + 1(type 0x03) + 2(name "a\0") + 5(embedded {}) + 1(doc term) + assertSkipValueSucceeds(new byte[]{ + 13, 0, 0, 0, // document size + typeByte(BsonType.DOCUMENT), + 0x61, 0x00, // name: "a\0" + 5, 0, 0, 0, // embedded document size: 5 (valid minimum) + 0x00, // embedded document terminator + 0x00 // outer document terminator + }); + } + + @Test + public void testSkipValueAcceptsMinimalArray() { + // {"a": []} - embedded array with size=5 (minimum valid: int32 size + '\x00') + // doc size=13: 4(docsize) + 1(type 0x04) + 2(name "a\0") + 5(embedded []) + 1(doc term) + assertSkipValueSucceeds(new byte[]{ + 13, 0, 0, 0, // document size + typeByte(BsonType.ARRAY), + 0x61, 0x00, // name: "a\0" + 5, 0, 0, 0, // embedded array size: 5 (valid minimum) + 0x00, // embedded array terminator + 0x00 // outer document terminator + }); + } + + @Test + public void testSkipValueAcceptsMinimalJavaScriptWithScope() { + // {"a": } + // doc size=22: 4(size) + 1(type 0x0F) + 2(name "a\0") + 14(js_ws value) + 1(doc term) + assertSkipValueSucceeds(new byte[]{ + 22, 0, 0, 0, // document size + typeByte(BsonType.JAVASCRIPT_WITH_SCOPE), + 0x61, 0x00, // name: "a\0" + 14, 0, 0, 0, // javascript_with_scope total size + 1, 0, 0, 0, // code string size: 1 + 0x00, // code string content: "\0" + 5, 0, 0, 0, // scope document size: 5 (valid minimum) + 0x00, // scope document terminator + 0x00 // outer document terminator + }); + } + + /** + * Wraps the given bytes in a BSON reader, advances past the document start, type, and field name, + * and asserts that {@code skipValue()} throws {@link BsonSerializationException}. + */ + private static void assertSkipValueThrows(final byte[] bytes) { + try (BsonBinaryReader reader = createReaderForBytes(bytes)) { + reader.readStartDocument(); + reader.readBsonType(); + reader.readName(); + assertThrows(BsonSerializationException.class, reader::skipValue); + } + } + + /** + * Wraps the given bytes in a BSON reader, advances past the document start, type, and field name, + * skips the value, and asserts the document terminates cleanly. + */ + private static void assertSkipValueSucceeds(final byte[] bytes) { + try (BsonBinaryReader reader = createReaderForBytes(bytes)) { + reader.readStartDocument(); + reader.readBsonType(); + reader.readName(); + reader.skipValue(); + reader.readEndDocument(); + } + } + + private static BsonBinaryReader createReaderForBytes(final byte[] bytes) { return new BsonBinaryReader(new ByteBufferBsonInput(new ByteBufNIO(ByteBuffer.wrap(bytes)))); } + + private static byte typeByte(final BsonType type) { + return (byte) type.getValue(); + } } diff --git a/bson/src/test/unit/org/bson/RawBsonDocumentTest.java b/bson/src/test/unit/org/bson/RawBsonDocumentTest.java index 866dc10179..1ad24bc02f 100644 --- a/bson/src/test/unit/org/bson/RawBsonDocumentTest.java +++ b/bson/src/test/unit/org/bson/RawBsonDocumentTest.java @@ -21,6 +21,7 @@ import org.bson.io.BasicOutputBuffer; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.api.Named; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.provider.Arguments; import org.junit.jupiter.params.provider.MethodSource; @@ -29,6 +30,8 @@ import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; class RawBsonDocumentTest { @@ -72,6 +75,67 @@ void shouldExposeBackingArrayOffsetAndLength(final RawBsonDocument rawDocument, rawDocument.getByteOffset() + rawDocument.getByteLength())); } + @Test + void sizeShouldRejectMalformedZeroLengthStringValue() { + RawBsonDocument malformed = createDocumentWithZeroLengthStringValue(); + assertThrows(BsonSerializationException.class, malformed::size); + } + + @Test + void containsKeyShouldRejectMalformedZeroLengthStringWhenKeyNotFound() { + // "nonExistent" forces iteration past the malformed "x" field via skipValue(). + RawBsonDocument malformed = createDocumentWithZeroLengthStringValue(); + assertThrows(BsonSerializationException.class, () -> malformed.containsKey("nonExistent")); + } + + @Test + void containsKeyShouldReturnTrueForMalformedFieldKey() { + // "x" is the malformed field itself - containsKey matches the key before skipValue() is called. + RawBsonDocument malformed = createDocumentWithZeroLengthStringValue(); + assertTrue(malformed.containsKey("x")); + } + + @Test + void getShouldRejectMalformedZeroLengthStringWhenKeyNotFound() { + // "nonExistent" forces iteration past the malformed "x" field via skipValue(). + RawBsonDocument malformed = createDocumentWithZeroLengthStringValue(); + assertThrows(BsonSerializationException.class, () -> malformed.get("nonExistent")); + } + + @Test + void getFirstKeyShouldSucceedForMalformedDocument() { + // getFirstKey() reads the key name but never calls skipValue(), so it succeeds + // even when the first field's value is malformed. + RawBsonDocument malformed = createDocumentWithZeroLengthStringValue(); + assertEquals("x", malformed.getFirstKey()); + } + + /** + * Creates a {@link RawBsonDocument} containing a string field whose declared size is zero. + * Encodes {"x": "hello", "a": "world"} on a cloned byte array, then corrupts the string size + * of "x" to 0. "x" is deliberately the first field so that {@code getFirstKey()} and + * {@code containsKey("x")} can short-circuit before the malformed value is reached. + */ + private static RawBsonDocument createDocumentWithZeroLengthStringValue() { + BsonDocument doc = new BsonDocument() + .append("x", new BsonString("hello")) + .append("a", new BsonString("world")); + RawBsonDocument raw = new RawBsonDocument(doc, new BsonDocumentCodec()); + // Clone so the malformed document does not share state with `raw`. + byte[] bytes = raw.getBackingArray().clone(); + int offset = raw.getByteOffset(); + // BSON layout: [4 docSize] [0x02 type] [x \0 name] [int32 stringSize] [string bytes] ... + // The string size field for "x" starts at offset + 4(docSize) + 1(type) + 2(name "x\0") = offset + 7. + int stringSizeOffset = offset + 7; + assertEquals(6, (bytes[stringSizeOffset] & 0xFF) + | ((bytes[stringSizeOffset + 1] & 0xFF) << 8) + | ((bytes[stringSizeOffset + 2] & 0xFF) << 16) + | ((bytes[stringSizeOffset + 3] & 0xFF) << 24), + "Expected test fixture string length 6 (\"hello\\0\") at offset+7; encoding assumptions may have changed"); + Arrays.fill(bytes, stringSizeOffset, stringSizeOffset + 4, (byte) 0); + return new RawBsonDocument(bytes, offset, raw.getByteLength()); + } + private static Named createFromDocument() { return Named.of("from document", new RawBsonDocument(DOCUMENT, new BsonDocumentCodec())); }