Fix cord handling in DynamicMessage and oneofs. (#18373)

* Fix cord handling in DynamicMessage and oneofs. This fixes a memory corruption vulnerability for anyone using cord with dynamically built descriptor pools. * Silence expected ubsan failures from absl::Cord --------- Co-authored-by: Mike Kruskal <[email protected]>
protocolbuffers · Sep 18, 2024 · 5b0e543 · 5b0e543
1 parent 421fc16
commit 5b0e543
Show file tree

Hide file tree

Showing 51 changed files with 672 additions and 527 deletions.
diff --git a/.bazelrc b/.bazelrc
@@ -22,6 +22,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1
 # Workaround for the fact that Bazel links with $CC, not $CXX
 # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748
 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr
+# Abseil passes nullptr to memcmp with 0 size
+build:ubsan --copt=-fno-sanitize=nonnull-attribute
 
 # TODO: migrate all dependencies from WORKSPACE to MODULE.bazel
 # https://github.com/protocolbuffers/protobuf/issues/14313

diff --git a/ci/common.bazelrc b/ci/common.bazelrc
@@ -27,6 +27,8 @@ build:ubsan --action_env=UBSAN_OPTIONS=halt_on_error=1:print_stacktrace=1
 # Workaround for the fact that Bazel links with $CC, not $CXX
 # https://github.com/bazelbuild/bazel/issues/11122#issuecomment-613746748
 build:ubsan --copt=-fno-sanitize=function --copt=-fno-sanitize=vptr
+# Abseil passes nullptr to memcmp with 0 size
+build:ubsan --copt=-fno-sanitize=nonnull-attribute
 
 # Workaround Bazel 7 remote cache issues.
 # See https://github.com/bazelbuild/bazel/issues/20161

diff --git a/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java b/java/core/src/test/java/com/google/protobuf/CodedOutputStreamTest.java
@@ -13,7 +13,6 @@
 import com.google.protobuf.CodedOutputStream.OutOfSpaceException;
 import protobuf_unittest.UnittestProto.SparseEnumMessage;
 import protobuf_unittest.UnittestProto.TestAllTypes;
-import protobuf_unittest.UnittestProto.TestPackedTypes;
 import protobuf_unittest.UnittestProto.TestSparseEnum;
 import java.io.ByteArrayInputStream;
 import java.io.ByteArrayOutputStream;
@@ -402,47 +401,6 @@ public void computeTagSize() {
     assertThat(CodedOutputStream.computeTagSize((1 << 30) + 1)).isEqualTo(1);
   }
 
-  /** Tests writing a whole message with every field type. */
-  @Test
-  public void testWriteWholeMessage() throws Exception {
-    final byte[] expectedBytes = TestUtil.getGoldenMessage().toByteArray();
-    TestAllTypes message = TestUtil.getAllSet();
-
-    for (OutputType outputType : OutputType.values()) {
-      Coder coder = outputType.newCoder(message.getSerializedSize());
-      message.writeTo(coder.stream());
-      coder.stream().flush();
-      byte[] rawBytes = coder.toByteArray();
-      assertEqualBytes(outputType, expectedBytes, rawBytes);
-    }
-
-    // Try different block sizes.
-    for (int blockSize = 1; blockSize < 256; blockSize *= 2) {
-      Coder coder = OutputType.STREAM.newCoder(blockSize);
-      message.writeTo(coder.stream());
-      coder.stream().flush();
-      assertEqualBytes(OutputType.STREAM, expectedBytes, coder.toByteArray());
-    }
-  }
-
-  /**
-   * Tests writing a whole message with every packed field type. Ensures the wire format of packed
-   * fields is compatible with C++.
-   */
-  @Test
-  public void testWriteWholePackedFieldsMessage() throws Exception {
-    byte[] expectedBytes = TestUtil.getGoldenPackedFieldsMessage().toByteArray();
-    TestPackedTypes message = TestUtil.getPackedSet();
-
-    for (OutputType outputType : OutputType.values()) {
-      Coder coder = outputType.newCoder(message.getSerializedSize());
-      message.writeTo(coder.stream());
-      coder.stream().flush();
-      byte[] rawBytes = coder.toByteArray();
-      assertEqualBytes(outputType, expectedBytes, rawBytes);
-    }
-  }
-
   /**
    * Test writing a message containing a negative enum value. This used to fail because the size was
    * not properly computed as a sign-extended varint.