Skip to content

Commit

Permalink
Check that size is non-negative when reading string or bytes in Strea…
Browse files Browse the repository at this point in the history
…mDecoder.

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
  • Loading branch information
protobuf-github-bot authored and Copybara-Service committed Apr 10, 2024
1 parent 9b8f41c commit a94f57b
Show file tree
Hide file tree
Showing 2 changed files with 94 additions and 0 deletions.
13 changes: 13 additions & 0 deletions java/core/src/main/java/com/google/protobuf/CodedInputStream.java
Original file line number Diff line number Diff line change
Expand Up @@ -2278,6 +2278,9 @@ public String readString() throws IOException {
if (size == 0) {
return "";
}
if (size < 0) {
throw InvalidProtocolBufferException.negativeSize();
}
if (size <= bufferSize) {
refillBuffer(size);
String result = new String(buffer, pos, size, UTF_8);
Expand All @@ -2302,6 +2305,8 @@ public String readStringRequireUtf8() throws IOException {
tempPos = oldPos;
} else if (size == 0) {
return "";
} else if (size < 0) {
throw InvalidProtocolBufferException.negativeSize();
} else if (size <= bufferSize) {
refillBuffer(size);
bytes = buffer;
Expand Down Expand Up @@ -2396,6 +2401,9 @@ public ByteString readBytes() throws IOException {
if (size == 0) {
return ByteString.EMPTY;
}
if (size < 0) {
throw InvalidProtocolBufferException.negativeSize();
}
return readBytesSlowPath(size);
}

Expand All @@ -2408,6 +2416,8 @@ public byte[] readByteArray() throws IOException {
final byte[] result = Arrays.copyOfRange(buffer, pos, pos + size);
pos += size;
return result;
} else if (size < 0) {
throw InvalidProtocolBufferException.negativeSize();
} else {
// Slow path: Build a byte array first then copy it.
// TODO: Do we want to protect from malicious input streams here?
Expand All @@ -2427,6 +2437,9 @@ public ByteBuffer readByteBuffer() throws IOException {
if (size == 0) {
return Internal.EMPTY_BYTE_BUFFER;
}
if (size < 0) {
throw InvalidProtocolBufferException.negativeSize();
}
// Slow path: Build a byte array first then copy it.

// We must copy as the byte array was handed off to the InputStream and a malicious
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import static com.google.common.truth.Truth.assertThat;
import static com.google.common.truth.Truth.assertWithMessage;
import static org.junit.Assert.assertArrayEquals;
import static org.junit.Assert.assertThrows;
import protobuf_unittest.UnittestProto.BoolMessage;
import protobuf_unittest.UnittestProto.Int32Message;
import protobuf_unittest.UnittestProto.Int64Message;
Expand Down Expand Up @@ -534,6 +535,86 @@ public void testReadMaliciouslyLargeBlob() throws Exception {
}
}

@Test
public void testReadStringWithSizeOverflow_throwsInvalidProtocolBufferException()
throws Exception {
ByteString.Output rawOutput = ByteString.newOutput();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);

output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
output.flush();
byte[] data = rawOutput.toByteString().toByteArray();
for (InputType inputType : InputType.values()) {
CodedInputStream input = inputType.newDecoder(data);
assertThrows(InvalidProtocolBufferException.class, input::readString);
}
}

@Test
public void testReadStringRequireUtf8WithSizeOverflow_throwsInvalidProtocolBufferException()
throws Exception {
ByteString.Output rawOutput = ByteString.newOutput();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);

output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
output.flush();
byte[] data = rawOutput.toByteString().toByteArray();
for (InputType inputType : InputType.values()) {
CodedInputStream input = inputType.newDecoder(data);
assertThrows(InvalidProtocolBufferException.class, input::readStringRequireUtf8);
}
}

@Test
public void testReadBytesWithHugeSizeOverflow_throwsInvalidProtocolBufferException()
throws Exception {
ByteString.Output rawOutput = ByteString.newOutput();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);

output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
output.flush();
byte[] data = rawOutput.toByteString().toByteArray();
for (InputType inputType : InputType.values()) {
CodedInputStream input = inputType.newDecoder(data);
assertThrows(InvalidProtocolBufferException.class, input::readBytes);
}
}

@Test
public void testReadByteArrayWithHugeSizeOverflow_throwsInvalidProtocolBufferException()
throws Exception {
ByteString.Output rawOutput = ByteString.newOutput();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);

output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
output.flush();
byte[] data = rawOutput.toByteString().toByteArray();
for (InputType inputType : InputType.values()) {
CodedInputStream input = inputType.newDecoder(data);
assertThrows(InvalidProtocolBufferException.class, input::readByteArray);
}
}

@Test
public void testReadByteBufferWithSizeOverflow_throwsInvalidProtocolBufferException()
throws Exception {
ByteString.Output rawOutput = ByteString.newOutput();
CodedOutputStream output = CodedOutputStream.newInstance(rawOutput);

output.writeUInt32NoTag(0xFFFFFFFF); // Larger than Integer.MAX_VALUE.
output.writeRawBytes(new byte[32]); // Pad with a few random bytes.
output.flush();
byte[] data = rawOutput.toByteString().toByteArray();
for (InputType inputType : InputType.values()) {
CodedInputStream input = inputType.newDecoder(data);
assertThrows(InvalidProtocolBufferException.class, input::readByteBuffer);
}
}

/**
* Test we can do messages that are up to CodedInputStream#DEFAULT_SIZE_LIMIT in size (2G or
* Integer#MAX_SIZE).
Expand Down

0 comments on commit a94f57b

Please sign in to comment.