Merge pull request #17514 from protocolbuffers/cp-25

Check that size is non-negative when reading string or bytes in Strea…
protocolbuffers · Jul 17, 2024 · fb0520e · fb0520e
2 parents 5b514a9 + bdb1f75
commit fb0520e
Show file tree

Hide file tree

Showing 3 changed files with 100 additions and 0 deletions.
diff --git a/.github/workflows/test_cpp.yml b/.github/workflows/test_cpp.yml
@@ -408,6 +408,12 @@ jobs:
           arch: ${{ matrix.windows-arch || 'x64' }}
           vsversion: ${{ matrix.vsversion }}
 
+      # Workaround for Abseil incompatibility with CMake 3.30 (b/352354235).
+      - name: Downgrade CMake
+        if: ${{ runner.os == 'Windows' }}
+        run: choco install cmake --version 3.29.6 --force
+        shell: bash
+
       # Workaround for incompatibility between gcloud and windows-2019 runners.
       - name: Install Python
         if: ${{ matrix.python-version }}

diff --git a/java/core/src/main/java/com/google/protobuf/CodedInputStream.java b/java/core/src/main/java/com/google/protobuf/CodedInputStream.java
@@ -2276,6 +2276,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);
@@ -2300,6 +2303,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;
@@ -2394,6 +2399,9 @@ public ByteString readBytes() throws IOException {
       if (size == 0) {
         return ByteString.EMPTY;
       }
+      if (size < 0) {
+        throw InvalidProtocolBufferException.negativeSize();
+      }
       return readBytesSlowPath(size);
     }
 
@@ -2406,6 +2414,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?
@@ -2425,6 +2435,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

diff --git a/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedInputStreamTest.java
@@ -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;
@@ -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).