Skip to content

Commit

Permalink
Fix cord handling in DynamicMessage and oneofs. (#18375)
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.

* Update staleness. Run using Bazel 6.3.2 docker image

* Silence expected ubsan failures from absl::Cord

---------

Co-authored-by: Mike Kruskal <mkruskal@google.com>
  • Loading branch information
zhangskz and mkruskal-google committed Sep 18, 2024
1 parent 8a60b65 commit e673479
Show file tree
Hide file tree
Showing 57 changed files with 785 additions and 478 deletions.
2 changes: 2 additions & 0 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ build:ubsan --copt=-DUNDEFINED_SANITIZER=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 @@ -34,3 +34,5 @@ build:ubsan --copt=-DUNDEFINED_SANITIZER=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
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 @@ -327,47 +326,6 @@ public void testEncodeZigZag() throws Exception {
.isEqualTo(-75123905439571256L);
}

/** 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 e673479

Please sign in to comment.