Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add poison pill to makeExtensionsImmutable method #20084

Merged
merged 1 commit into from
Jan 23, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions java/core/BUILD.bazel
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ junit_tests(
"src/test/java/com/google/protobuf/IsValidUtf8Test.java",
"src/test/java/com/google/protobuf/TestUtil.java",
"src/test/java/com/google/protobuf/TestUtilLite.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
],
),
data = ["//src/google/protobuf:testdata"],
Expand Down Expand Up @@ -471,6 +472,7 @@ LITE_TEST_EXCLUSIONS = [
"src/test/java/com/google/protobuf/ExtensionRegistryFactoryTest.java",
"src/test/java/com/google/protobuf/FieldPresenceTest.java",
"src/test/java/com/google/protobuf/ForceFieldBuildersPreRun.java",
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
"src/test/java/com/google/protobuf/GeneratedMessageTest.java",
"src/test/java/com/google/protobuf/LazyFieldTest.java",
"src/test/java/com/google/protobuf/LazyStringEndToEndTest.java",
Expand Down Expand Up @@ -524,6 +526,25 @@ junit_tests(
],
)


java_test(
name = "GeneratedMessagePre22WarningDisabledTest",
size = "small",
srcs = [
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
],
jvm_flags = ["-Dcom.google.protobuf.use_unsafe_pre22_gencode"],
deps = [
":core",
":generic_test_protos_java_proto",
":java_test_protos_java_proto",
":lite_test_protos_java_proto",
":test_util",
"@maven//:com_google_truth_truth",
"@maven//:junit_junit",
],
)

pkg_files(
name = "dist_files",
srcs = glob([
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -310,9 +310,27 @@ public int getSerializedSize() {
return memoizedSize;
}

static final String PRE22_GENCODE_ACKNOWLEGE_PROPERTY =
"com.google.protobuf.use_unsafe_pre22_gencode";
static final String PRE22_GENCODE_VULNERABILITY_MESSAGE =
"As of 2022/09/29 (release 21.7) makeExtensionsImmutable should not be called from protobuf"
+ " gencode. If you are seeing this message, your gencode is vulnerable to a denial of"
+ " service attack. You should regenerate your code using protobuf 25.6 or later. Use the"
+ " latest version that meets your needs. However, if you understand the risks and wish"
+ " to continue with vulnerable gencode, you can set the system property"
+ " `-Dcom.google.protobuf.use_unsafe_pre22_gencode` on the command line. See security"
+ " vulnerability:"
+ " https://github.com/protocolbuffers/protobuf/security/advisories/GHSA-h4h5-3hr4-j3g2";

static void warnPre22Gencode() {
if (System.getProperty(PRE22_GENCODE_ACKNOWLEGE_PROPERTY) == null) {
throw new UnsupportedOperationException(PRE22_GENCODE_VULNERABILITY_MESSAGE);
}
}

/** Used by parsing constructors in generated classes. */
protected void makeExtensionsImmutable() {
// Noop for messages without extensions.
warnPre22Gencode();
}

/**
Expand Down Expand Up @@ -902,6 +920,7 @@ protected boolean parseUnknownField(
/** Used by parsing constructors in generated classes. */
@Override
protected void makeExtensionsImmutable() {
warnPre22Gencode();
extensions.makeImmutable();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -528,6 +528,7 @@ protected Object newInstance(UnusedPrivateParameter unused) {
*/
protected void makeExtensionsImmutable() {
// Noop for messages without extensions.
GeneratedMessage.warnPre22Gencode();
}

/**
Expand Down Expand Up @@ -1275,6 +1276,7 @@ protected boolean parseUnknownFieldProto3(
*/
@Override
protected void makeExtensionsImmutable() {
GeneratedMessage.warnPre22Gencode();
extensions.makeImmutable();
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
package com.google.protobuf;

import protobuf_unittest.UnittestProto.TestAllExtensions;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;

@RunWith(JUnit4.class)
public class GeneratedMessagePre22WarningDisabledTest {
@Test
public void generatedMessage_makeExtensionsImmutableShouldNotThrow() {
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
protected FieldAccessorTable internalGetFieldAccessorTable() {
return null;
}

@Override
protected Message.Builder newBuilderForType(BuilderParent parent) {
return null;
}

@Override
public Message.Builder newBuilderForType() {
return null;
}

@Override
public Message.Builder toBuilder() {
return null;
}

@Override
public Message getDefaultInstanceForType() {
return null;
}
};
msg.makeExtensionsImmutable();
}

@Test
public void extendableMessage_makeExtensionsImmutableShouldNotThrow() {
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
TestAllExtensions.newBuilder().build();
msg.makeExtensionsImmutable();
}
}

Original file line number Diff line number Diff line change
Expand Up @@ -1999,4 +1999,57 @@ public void extendableBuilder_mergeFrom_repeatedField_doesNotInvalidateExistingB
assertThat(builder.getRepeatedField(REPEATED_NESTED_MESSAGE_EXTENSION, 0))
.isEqualTo(NestedMessage.newBuilder().setBb(100).build());
}

@Test
public void generatedMessage_makeExtensionsImmutableShouldThrow() {
GeneratedMessageV3 msg =
new GeneratedMessageV3() {
@Override
protected FieldAccessorTable internalGetFieldAccessorTable() {
return null;
}

@Override
protected Message.Builder newBuilderForType(BuilderParent parent) {
return null;
}

@Override
public Message.Builder newBuilderForType() {
return null;
}

@Override
public Message.Builder toBuilder() {
return null;
}

@Override
public Message getDefaultInstanceForType() {
return null;
}
};
try {
msg.makeExtensionsImmutable();
assertWithMessage("Expected UnsupportedOperationException").fail();
} catch (UnsupportedOperationException e) {
// Expected
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
}
}

@Test
public void extendableMessage_makeExtensionsImmutableShouldThrow() {
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
TestAllExtensions.getDefaultInstance();
try {
msg.makeExtensionsImmutable();
assertWithMessage("Expected UnsupportedOperationException").fail();
} catch (UnsupportedOperationException e) {
// Expected
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
}
}
}
Loading