Skip to content

Commit

Permalink
Fix cord handling in DynamicMessage and oneofs. (#18373)
Browse files Browse the repository at this point in the history
* 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 <mkruskal@google.com>
  • Loading branch information
zhangskz and mkruskal-google authored Sep 18, 2024
1 parent 421fc16 commit 5b0e543
Show file tree
Hide file tree
Showing 51 changed files with 672 additions and 527 deletions.
2 changes: 2 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 2 additions & 0 deletions ci/common.bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
Loading

0 comments on commit 5b0e543

Please sign in to comment.