This is an automated email from the ASF dual-hosted git repository. jamesbognar pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/juneau.git
commit 694bed1ddc02f26d5dbe9137910fbec157c7acee Author: James Bognar <[email protected]> AuthorDate: Sun Aug 16 15:33:06 2026 -0400 READY-388/391: Bound Protobuf LEN fields by maxLength and reject overlong varints --- .../juneau/marshall/protobuf/ProtobufReader.java | 37 ++++++++++++++++--- .../marshall/protobuf/ProtobufReader_Test.java | 42 ++++++++++++++++++++++ 2 files changed, 75 insertions(+), 4 deletions(-) diff --git a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java index 4d52c8662e..e7599c174a 100644 --- a/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java +++ b/juneau-core/juneau-marshall/src/main/java/org/apache/juneau/marshall/protobuf/ProtobufReader.java @@ -57,6 +57,9 @@ public class ProtobufReader { /** Default cap (16 MiB) for wire-declared length-delimited block sizes. */ static final int DEFAULT_MAX_LENGTH = 16 * 1024 * 1024; + /** Maximum encoded length, in bytes, of a 64-bit protobuf varint (ceil(64 / 7) LEB128 groups). */ + private static final int MAX_VARINT_BYTES = 10; + private final InputStream is; private int maxLength = DEFAULT_MAX_LENGTH; @@ -112,19 +115,28 @@ public class ProtobufReader { /** * Reads a base-128 varint (LEB128) as a 64-bit value. * + * <p> + * Caps the encoding at {@value #MAX_VARINT_BYTES} bytes (the maximum a 64-bit varint can occupy), so a + * malformed stream of continuation bytes cannot burn unbounded CPU/bytes before EOF. + * * @return The decoded value. - * @throws IOException If the stream ends mid-varint or the underlying stream fails. + * @throws IOException If the stream ends mid-varint, the varint exceeds {@value #MAX_VARINT_BYTES} bytes, + * or the underlying stream fails. */ public long readVarint() throws IOException { var result = 0L; var shift = 0; + var count = 0; int b; do { b = read(); if (b == -1) throw ioex("Unexpected end of protobuf input while reading varint"); + count++; result |= ((long)(b & 0x7F)) << shift; shift += 7; + if (count == MAX_VARINT_BYTES && (b & 0x80) != 0) + throw ioex("Protobuf varint exceeds maximum length of %s bytes", MAX_VARINT_BYTES); } while ((b & 0x80) != 0); return result; } @@ -132,8 +144,12 @@ public class ProtobufReader { /** * Reads a field tag, or returns {@link #EOF} if the stream is exhausted at a field boundary. * + * <p> + * Caps the encoding at {@value #MAX_VARINT_BYTES} bytes, the same bound applied by {@link #readVarint()}. + * * @return The decoded tag value (<c>(fieldNumber << 3) | wireType</c>), or {@link #EOF} at end of stream. - * @throws IOException If the stream ends mid-tag or the underlying stream fails. + * @throws IOException If the stream ends mid-tag, the tag exceeds {@value #MAX_VARINT_BYTES} bytes, or the + * underlying stream fails. */ public long readTag() throws IOException { var b = read(); @@ -141,12 +157,16 @@ public class ProtobufReader { return EOF; var result = (long)(b & 0x7F); var shift = 7; + var count = 1; while ((b & 0x80) != 0) { b = read(); if (b == -1) throw ioex("Unexpected end of protobuf input while reading tag"); + count++; result |= ((long)(b & 0x7F)) << shift; shift += 7; + if (count == MAX_VARINT_BYTES && (b & 0x80) != 0) + throw ioex("Protobuf varint exceeds maximum length of %s bytes", MAX_VARINT_BYTES); } return result; } @@ -277,15 +297,24 @@ public class ProtobufReader { /** * Skips a field's value, consuming exactly the right number of bytes for the specified wire type. * + * <p> + * For {@link WireType#LEN}, the wire-declared length is validated against the configured maximum (the + * same check applied by {@link #readLenDelimited()}) before skipping, so an unknown forward-compatible + * field cannot bypass the length cap via an unbounded or wrapped-negative skip. + * * @param wireType The wire type of the field to skip. - * @throws IOException If the stream ends early or the underlying stream fails. + * @throws IOException If the stream ends early, the declared length is out of bounds, or the underlying + * stream fails. */ public void skipField(WireType wireType) throws IOException { switch (wireType) { case VARINT -> readVarint(); case I64 -> skip(8); case I32 -> skip(4); - case LEN -> skip((int)readVarint()); + case LEN -> { + var len = ParserInputStream.checkLength(readVarint(), maxLength, "protobuf field"); + skip(len); + } default -> throw ioex("Cannot skip unsupported protobuf wire type: %s", wireType); } } diff --git a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java index 13f364fa9c..cc215deb12 100644 --- a/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java +++ b/juneau-core/juneau-marshall/src/test/java/org/apache/juneau/marshall/protobuf/ProtobufReader_Test.java @@ -16,8 +16,11 @@ */ package org.apache.juneau.marshall.protobuf; +import static org.apache.juneau.BasicTestUtils.*; import static org.junit.jupiter.api.Assertions.*; +import java.io.*; + import org.apache.juneau.*; import org.junit.jupiter.api.*; @@ -111,4 +114,43 @@ class ProtobufReader_Test extends TestBase { r4.skipField(WireType.LEN); assertEquals(2, ProtobufReader.fieldNumber(r4.readTag())); } + + @Test + void a09_varintRejectsOverlongContinuation() throws Exception { + // 10 continuation bytes (0x80) with no terminating byte: the 10th byte still has the continuation + // bit set, so the varint never terminates within the 10-byte cap. + var r = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80); + assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r::readVarint); + + // Same shape but with an 11th byte present ("10x0x80 + more") -- still rejected at byte 10. + var r2 = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x01); + assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r2::readVarint); + } + + @Test + void a10_varintCanonicalTenByteNegativeOneStillDecodes() throws Exception { + // Canonical 10-byte encoding of -1L must remain valid (terminating byte on the 10th). + assertEquals(-1L, reader(0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x01).readVarint()); + } + + @Test + void a11_tagRejectsOverlongContinuation() throws Exception { + var r = reader(0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80,0x80); + assertThrowsWithMessage(IOException.class, "exceeds maximum length of 10 bytes", r::readTag); + } + + @Test + void b01_skipFieldLenRejectsLengthAboveMaxLength() throws Exception { + // Declared length (10) exceeds the configured maximum (5); must reject before skipping any data. + var r = reader(0x0A, 1,2,3,4,5,6,7,8,9,10, 0x10); + r.setMaxLength(5); + assertThrowsWithMessage(IOException.class, "exceeds maximum allowed", () -> r.skipField(WireType.LEN)); + } + + @Test + void b02_skipFieldLenRejectsWrappedNegativeVarint() throws Exception { + // Varint decodes to -1L (wrapped-negative as an int cast); must reject rather than skip a negative count. + var r = reader(0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0xFF,0x01); + assertThrowsWithMessage(IOException.class, "negative", () -> r.skipField(WireType.LEN)); + } }
