Skip to content

Commit 3f1df78

Browse files
authored
Add poison pill to makeExtensionsImmutable method (#20084)
1 parent 156b87b commit 3f1df78

File tree

5 files changed

+145
-1
lines changed

5 files changed

+145
-1
lines changed

java/core/BUILD.bazel

+21
Original file line numberDiff line numberDiff line change
@@ -368,6 +368,7 @@ junit_tests(
368368
"src/test/java/com/google/protobuf/IsValidUtf8Test.java",
369369
"src/test/java/com/google/protobuf/TestUtil.java",
370370
"src/test/java/com/google/protobuf/TestUtilLite.java",
371+
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
371372
],
372373
),
373374
data = ["//src/google/protobuf:testdata"],
@@ -471,6 +472,7 @@ LITE_TEST_EXCLUSIONS = [
471472
"src/test/java/com/google/protobuf/ExtensionRegistryFactoryTest.java",
472473
"src/test/java/com/google/protobuf/FieldPresenceTest.java",
473474
"src/test/java/com/google/protobuf/ForceFieldBuildersPreRun.java",
475+
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
474476
"src/test/java/com/google/protobuf/GeneratedMessageTest.java",
475477
"src/test/java/com/google/protobuf/LazyFieldTest.java",
476478
"src/test/java/com/google/protobuf/LazyStringEndToEndTest.java",
@@ -524,6 +526,25 @@ junit_tests(
524526
],
525527
)
526528

529+
530+
java_test(
531+
name = "GeneratedMessagePre22WarningDisabledTest",
532+
size = "small",
533+
srcs = [
534+
"src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java",
535+
],
536+
jvm_flags = ["-Dcom.google.protobuf.use_unsafe_pre22_gencode"],
537+
deps = [
538+
":core",
539+
":generic_test_protos_java_proto",
540+
":java_test_protos_java_proto",
541+
":lite_test_protos_java_proto",
542+
":test_util",
543+
"@maven//:com_google_truth_truth",
544+
"@maven//:junit_junit",
545+
],
546+
)
547+
527548
pkg_files(
528549
name = "dist_files",
529550
srcs = glob([

java/core/src/main/java/com/google/protobuf/GeneratedMessage.java

+20-1
Original file line numberDiff line numberDiff line change
@@ -310,9 +310,27 @@ public int getSerializedSize() {
310310
return memoizedSize;
311311
}
312312

313+
static final String PRE22_GENCODE_ACKNOWLEGE_PROPERTY =
314+
"com.google.protobuf.use_unsafe_pre22_gencode";
315+
static final String PRE22_GENCODE_VULNERABILITY_MESSAGE =
316+
"As of 2022/09/29 (release 21.7) makeExtensionsImmutable should not be called from protobuf"
317+
+ " gencode. If you are seeing this message, your gencode is vulnerable to a denial of"
318+
+ " service attack. You should regenerate your code using protobuf 25.6 or later. Use the"
319+
+ " latest version that meets your needs. However, if you understand the risks and wish"
320+
+ " to continue with vulnerable gencode, you can set the system property"
321+
+ " `-Dcom.google.protobuf.use_unsafe_pre22_gencode` on the command line. See security"
322+
+ " vulnerability:"
323+
+ " https://github.com/protocolbuffers/protobuf/security/advisories/GHSA-h4h5-3hr4-j3g2";
324+
325+
static void warnPre22Gencode() {
326+
if (System.getProperty(PRE22_GENCODE_ACKNOWLEGE_PROPERTY) == null) {
327+
throw new UnsupportedOperationException(PRE22_GENCODE_VULNERABILITY_MESSAGE);
328+
}
329+
}
330+
313331
/** Used by parsing constructors in generated classes. */
314332
protected void makeExtensionsImmutable() {
315-
// Noop for messages without extensions.
333+
warnPre22Gencode();
316334
}
317335

318336
/**
@@ -902,6 +920,7 @@ protected boolean parseUnknownField(
902920
/** Used by parsing constructors in generated classes. */
903921
@Override
904922
protected void makeExtensionsImmutable() {
923+
warnPre22Gencode();
905924
extensions.makeImmutable();
906925
}
907926

java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java

+2
Original file line numberDiff line numberDiff line change
@@ -528,6 +528,7 @@ protected Object newInstance(UnusedPrivateParameter unused) {
528528
*/
529529
protected void makeExtensionsImmutable() {
530530
// Noop for messages without extensions.
531+
GeneratedMessage.warnPre22Gencode();
531532
}
532533

533534
/**
@@ -1275,6 +1276,7 @@ protected boolean parseUnknownFieldProto3(
12751276
*/
12761277
@Override
12771278
protected void makeExtensionsImmutable() {
1279+
GeneratedMessage.warnPre22Gencode();
12781280
extensions.makeImmutable();
12791281
}
12801282

Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
package com.google.protobuf;
2+
3+
import protobuf_unittest.UnittestProto.TestAllExtensions;
4+
import org.junit.Test;
5+
import org.junit.runner.RunWith;
6+
import org.junit.runners.JUnit4;
7+
8+
@RunWith(JUnit4.class)
9+
public class GeneratedMessagePre22WarningDisabledTest {
10+
@Test
11+
public void generatedMessage_makeExtensionsImmutableShouldNotThrow() {
12+
GeneratedMessageV3 msg =
13+
new GeneratedMessageV3() {
14+
@Override
15+
protected FieldAccessorTable internalGetFieldAccessorTable() {
16+
return null;
17+
}
18+
19+
@Override
20+
protected Message.Builder newBuilderForType(BuilderParent parent) {
21+
return null;
22+
}
23+
24+
@Override
25+
public Message.Builder newBuilderForType() {
26+
return null;
27+
}
28+
29+
@Override
30+
public Message.Builder toBuilder() {
31+
return null;
32+
}
33+
34+
@Override
35+
public Message getDefaultInstanceForType() {
36+
return null;
37+
}
38+
};
39+
msg.makeExtensionsImmutable();
40+
}
41+
42+
@Test
43+
public void extendableMessage_makeExtensionsImmutableShouldNotThrow() {
44+
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
45+
TestAllExtensions.newBuilder().build();
46+
msg.makeExtensionsImmutable();
47+
}
48+
}
49+

java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java

+53
Original file line numberDiff line numberDiff line change
@@ -1999,4 +1999,57 @@ public void extendableBuilder_mergeFrom_repeatedField_doesNotInvalidateExistingB
19991999
assertThat(builder.getRepeatedField(REPEATED_NESTED_MESSAGE_EXTENSION, 0))
20002000
.isEqualTo(NestedMessage.newBuilder().setBb(100).build());
20012001
}
2002+
2003+
@Test
2004+
public void generatedMessage_makeExtensionsImmutableShouldThrow() {
2005+
GeneratedMessageV3 msg =
2006+
new GeneratedMessageV3() {
2007+
@Override
2008+
protected FieldAccessorTable internalGetFieldAccessorTable() {
2009+
return null;
2010+
}
2011+
2012+
@Override
2013+
protected Message.Builder newBuilderForType(BuilderParent parent) {
2014+
return null;
2015+
}
2016+
2017+
@Override
2018+
public Message.Builder newBuilderForType() {
2019+
return null;
2020+
}
2021+
2022+
@Override
2023+
public Message.Builder toBuilder() {
2024+
return null;
2025+
}
2026+
2027+
@Override
2028+
public Message getDefaultInstanceForType() {
2029+
return null;
2030+
}
2031+
};
2032+
try {
2033+
msg.makeExtensionsImmutable();
2034+
assertWithMessage("Expected UnsupportedOperationException").fail();
2035+
} catch (UnsupportedOperationException e) {
2036+
// Expected
2037+
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
2038+
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
2039+
}
2040+
}
2041+
2042+
@Test
2043+
public void extendableMessage_makeExtensionsImmutableShouldThrow() {
2044+
GeneratedMessageV3.ExtendableMessage<TestAllExtensions> msg =
2045+
TestAllExtensions.getDefaultInstance();
2046+
try {
2047+
msg.makeExtensionsImmutable();
2048+
assertWithMessage("Expected UnsupportedOperationException").fail();
2049+
} catch (UnsupportedOperationException e) {
2050+
// Expected
2051+
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_VULNERABILITY_MESSAGE);
2052+
assertThat(e).hasMessageThat().contains(GeneratedMessage.PRE22_GENCODE_ACKNOWLEGE_PROPERTY);
2053+
}
2054+
}
20022055
}

0 commit comments

Comments
 (0)