Add poison pill to makeExtensionsImmutable method (#20084)

protocolbuffers · Jan 23, 2025 · 3f1df78 · 3f1df78
1 parent 156b87b
commit 3f1df78
Show file tree

Hide file tree

Showing 5 changed files with 145 additions and 1 deletion.
diff --git a/java/core/BUILD.bazel b/java/core/BUILD.bazel
@@ -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"],
@@ -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",
@@ -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([

diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessage.java
@@ -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();
   }
 
   /**
@@ -902,6 +920,7 @@ protected boolean parseUnknownField(
     /** Used by parsing constructors in generated classes. */
     @Override
     protected void makeExtensionsImmutable() {
+      warnPre22Gencode();
       extensions.makeImmutable();
     }
 

diff --git a/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java b/java/core/src/main/java/com/google/protobuf/GeneratedMessageV3.java
@@ -528,6 +528,7 @@ protected Object newInstance(UnusedPrivateParameter unused) {
    */
   protected void makeExtensionsImmutable() {
     // Noop for messages without extensions.
+    GeneratedMessage.warnPre22Gencode();
   }
 
   /**
@@ -1275,6 +1276,7 @@ protected boolean parseUnknownFieldProto3(
      */
     @Override
     protected void makeExtensionsImmutable() {
+      GeneratedMessage.warnPre22Gencode();
       extensions.makeImmutable();
     }
 

diff --git a/java/core/src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java b/java/core/src/test/java/com/google/protobuf/GeneratedMessagePre22WarningDisabledTest.java
@@ -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();
+  }
+}
+
diff --git a/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java b/java/core/src/test/java/com/google/protobuf/GeneratedMessageTest.java
@@ -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);
+    }
+  }
 }