Skip to content

Commit a94f57b

Browse files
Check that size is non-negative when reading string or bytes in StreamDecoder.
This ensures that StreamDecoder throws a InvalidProtocolBufferException instead of an IllegalStateException on some invalid input. All other implementations of CodedInputStream already do this check. PiperOrigin-RevId: 623383287
1 parent 9b8f41c commit a94f57b

File tree

2 files changed

+94
-0
lines changed

2 files changed

+94
-0
lines changed

java/core/src/main/java/com/google/protobuf/CodedInputStream.java

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2278,6 +2278,9 @@ public String readString() throws IOException {
22782278
if (size == 0) {
22792279
return "";
22802280
}
2281+
if (size < 0) {
2282+
throw InvalidProtocolBufferException.negativeSize();
2283+
}
22812284
if (size <= bufferSize) {
22822285
refillBuffer(size);
22832286
String result = new String(buffer, pos, size, UTF_8);
@@ -2302,6 +2305,8 @@ public String readStringRequireUtf8() throws IOException {
23022305
tempPos = oldPos;
23032306
} else if (size == 0) {
23042307
return "";
2308+
} else if (size < 0) {
2309+
throw InvalidProtocolBufferException.negativeSize();
23052310
} else if (size <= bufferSize) {
23062311
refillBuffer(size);
23072312
bytes = buffer;
@@ -2396,6 +2401,9 @@ public ByteString readBytes() throws IOException {
23962401
if (size == 0) {
23972402
return ByteString.EMPTY;
23982403
}
2404+
if (size < 0) {
2405+
throw InvalidProtocolBufferException.negativeSize();
2406+
}
23992407
return readBytesSlowPath(size);
24002408
}
24012409

@@ -2408,6 +2416,8 @@ public byte[] readByteArray() throws IOException {
24082416
final byte[] result = Arrays.copyOfRange(buffer, pos, pos + size);
24092417
pos += size;
24102418
return result;
2419+
} else if (size < 0) {
2420+
throw InvalidProtocolBufferException.negativeSize();
24112421
} else {
24122422
// Slow path: Build a byte array first then copy it.
24132423
// TODO: Do we want to protect from malicious input streams here?
@@ -2427,6 +2437,9 @@ public ByteBuffer readByteBuffer() throws IOException {
24272437
if (size == 0) {
24282438
return Internal.EMPTY_BYTE_BUFFER;
24292439
}
2440+
if (size < 0) {
2441+
throw InvalidProtocolBufferException.negativeSize();
2442+
}
24302443
// Slow path: Build a byte array first then copy it.
24312444

24322445
// We must copy as the byte array was handed off to the InputStream and a malicious

java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import static com.google.common.truth.Truth.assertThat;
1111
import static com.google.common.truth.Truth.assertWithMessage;
1212
import static org.junit.Assert.assertArrayEquals;
13+
import static org.junit.Assert.assertThrows;
1314
import protobuf_unittest.UnittestProto.BoolMessage;
1415
import protobuf_unittest.UnittestProto.Int32Message;
1516
import protobuf_unittest.UnittestProto.Int64Message;
@@ -534,6 +535,86 @@ public void testReadMaliciouslyLargeBlob() throws Exception {
534535
}
535536
}
536537

538+
@Test
539+
public void testReadStringWithSizeOverflow_throwsInvalidProtocolBufferException()
540+
throws Exception {
541+
ByteString.Output rawOutput = ByteString.newOutput();
542+
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);
543+
544+
output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
545+
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
546+
output.flush();
547+
byte[] data = rawOutput.toByteString().toByteArray();
548+
for (InputType inputType : InputType.values()) {
549+
CodedInputStream input = inputType.newDecoder(data);
550+
assertThrows(InvalidProtocolBufferException.class, input::readString);
551+
}
552+
}
553+
554+
@Test
555+
public void testReadStringRequireUtf8WithSizeOverflow_throwsInvalidProtocolBufferException()
556+
throws Exception {
557+
ByteString.Output rawOutput = ByteString.newOutput();
558+
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);
559+
560+
output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
561+
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
562+
output.flush();
563+
byte[] data = rawOutput.toByteString().toByteArray();
564+
for (InputType inputType : InputType.values()) {
565+
CodedInputStream input = inputType.newDecoder(data);
566+
assertThrows(InvalidProtocolBufferException.class, input::readStringRequireUtf8);
567+
}
568+
}
569+
570+
@Test
571+
public void testReadBytesWithHugeSizeOverflow_throwsInvalidProtocolBufferException()
572+
throws Exception {
573+
ByteString.Output rawOutput = ByteString.newOutput();
574+
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);
575+
576+
output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
577+
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
578+
output.flush();
579+
byte[] data = rawOutput.toByteString().toByteArray();
580+
for (InputType inputType : InputType.values()) {
581+
CodedInputStream input = inputType.newDecoder(data);
582+
assertThrows(InvalidProtocolBufferException.class, input::readBytes);
583+
}
584+
}
585+
586+
@Test
587+
public void testReadByteArrayWithHugeSizeOverflow_throwsInvalidProtocolBufferException()
588+
throws Exception {
589+
ByteString.Output rawOutput = ByteString.newOutput();
590+
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);
591+
592+
output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
593+
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
594+
output.flush();
595+
byte[] data = rawOutput.toByteString().toByteArray();
596+
for (InputType inputType : InputType.values()) {
597+
CodedInputStream input = inputType.newDecoder(data);
598+
assertThrows(InvalidProtocolBufferException.class, input::readByteArray);
599+
}
600+
}
601+
602+
@Test
603+
public void testReadByteBufferWithSizeOverflow_throwsInvalidProtocolBufferException()
604+
throws Exception {
605+
ByteString.Output rawOutput = ByteString.newOutput();
606+
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);
607+
608+
output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
609+
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
610+
output.flush();
611+
byte[] data = rawOutput.toByteString().toByteArray();
612+
for (InputType inputType : InputType.values()) {
613+
CodedInputStream input = inputType.newDecoder(data);
614+
assertThrows(InvalidProtocolBufferException.class, input::readByteBuffer);
615+
}
616+
}
617+
537618
/**
538619
* Test we can do messages that are up to CodedInputStream#DEFAULT_SIZE_LIMIT in size (2G or
539620
* Integer#MAX_SIZE).

0 commit comments

Comments
 (0)