CVE-2022-3509: Insufficient Information (NVD-CWE-noinfo)
value: A parsing issue similar to CVE-2022-3171, but with textformat in protobuf-java core and lite versions prior to 3.21.7, 3.20.3, 3.19.6 and 3.16.3 can lead to a denial of service attack. Inputs containing multiple instances of non-repeated embedded messages with repeated or unknown fields causes objects to be converted back-n-forth between mutable and immutable forms, resulting in potentially long garbage collection pauses. We recommend updating to the versions mentioned above.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
CVSS v3 base score: 7.5
Published: 2022-12-12T13:15Z
Repository: https://github.com/protocolbuffers/protobuf
Language: Java
Files changed: CHANGES.txt, java/core/src/main/java/com/google/protobuf/MessageReflection.java
Commit date: 2022-09-29 10:09:39 -0700
Commit message: Clean up TextFormat parser (#10673)

* Fix TextFormat parser
Diff: diff --git a/CHANGES.txt b/CHANGES.txt
index cf9c341e8..ba3e277ae 100644
--- a/CHANGES.txt
+++ b/CHANGES.txt
@@ -5,11 +5,15 @@
   * Move proto wireformat parsing functionality from the private "parsing
     constructor" to the Builder class.
   * Change the Lite runtime to prefer merging from the wireformat into mutable
     messages rather than building up a new immutable object before merging. This
     way results in fewer allocations and copy operations.
-  * Make message-type extensions merge from wire-format instead of building up instances and merging afterwards. This has much better performance.
+  * Make message-type extensions merge from wire-format instead of building up
+    instances and merging afterwards. This has much better performance.
+  * Fix TextFormat parser to build up recurring (but supposedly not repeated)
+    sub-messages directly from text rather than building a new sub-message and
+    merging the fully formed message into the existing field.
 
 2022-09-13 version 21.6 (C++/Java/Python/PHP/Objective-C/C#/Ruby)
 
 C++
 * Reduce memory consumption of MessageSet parsing
diff --git a/java/core/src/main/java/com/google/protobuf/MessageReflection.java b/java/core/src/main/java/com/google/protobuf/MessageReflection.java
index e9bc9f133..294d84177 100644
--- a/java/core/src/main/java/com/google/protobuf/MessageReflection.java
+++ b/java/core/src/main/java/com/google/protobuf/MessageReflection.java
@@ -376,10 +376,11 @@ class MessageReflection {
   }
 
   static class BuilderAdapter implements MergeTarget {
 
     private final Message.Builder builder;
+    private boolean hasNestedBuilders = true;
 
     @Override
     public Descriptors.Descriptor getDescriptorForType() {
       return builder.getDescriptorForType();
     }
@@ -391,17 +392,34 @@ class MessageReflection {
     @Override
     public Object getField(Descriptors.FieldDescriptor field) {
       return builder.getField(field);
     }
 
+    private Message.Builder getFieldBuilder(Descriptors.FieldDescriptor field) {
+      if (hasNestedBuilders) {
+        try {
+          return builder.getFieldBuilder(field);
+        } catch (UnsupportedOperationException e) {
+          hasNestedBuilders = false;
+        }
+      }
+      return null;
+    }
+
     @Override
     public boolean hasField(Descriptors.FieldDescriptor field) {
       return builder.hasField(field);
     }
 
     @Override
     public MergeTarget setField(Descriptors.FieldDescriptor field, Object value) {
+      if (!field.isRepeated() && value instanceof MessageLite.Builder) {
+        if (value != getFieldBuilder(field)) {
+          builder.setField(field, ((MessageLite.Builder) value).buildPartial());
+        }
+        return this;
+      }
       builder.setField(field, value);
       return this;
     }
 
     @Override
@@ -411,16 +429,22 @@ class MessageReflection {
     }
 
     @Override
     public MergeTarget setRepeatedField(
         Descriptors.FieldDescriptor field, int index, Object value) {
+      if (value instanceof MessageLite.Builder) {
+        value = ((MessageLite.Builder) value).buildPartial();
+      }
       builder.setRepeatedField(field, index, value);
       return this;
     }
 
     @Override
     public MergeTarget addRepeatedField(Descriptors.FieldDescriptor field, Object value) {
+      if (value instanceof MessageLite.Builder) {
+        value = ((MessageLite.Builder) value).buildPartial();
+      }
       builder.addRepeatedField(field, value);
       return this;
     }
 
     @Override
@@ -534,15 +558,23 @@ class MessageReflection {
         ExtensionRegistryLite extensionRegistry,
         FieldDescriptor field,
         Message defaultInstance)
         throws IOException {
       if (!field.isRepeated()) {
+        Message.Builder subBuilder;
         if (hasField(field)) {
-          input.readGroup(field.getNumber(), builder.getFieldBuilder(field), extensionRegistry);
-          return;
+          subBuilder = getFieldBuilder(field);
+          if (subBuilder != null) {
+            input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
+            return;
+          } else {
+            subBuilder = newMessageFieldInstance(field, defaultInstance);
+            subBuilder.mergeFrom((Message) getField(field));
+          }
+        } else {
+          subBuilder = newMessageFieldInstance(field, defaultInstance);
         }
-        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
         Object unused = setField(field, subBuilder.buildPartial());
       } else {
         Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
@@ -556,15 +588,23 @@ class MessageReflection {
         ExtensionRegistryLite extensionRegistry,
         Descriptors.FieldDescriptor field,
         Message defaultInstance)
         throws IOException {
       if (!field.isRepeated()) {
+        Message.Builder subBuilder;
         if (hasField(field)) {
-          input.readMessage(builder.getFieldBuilder(field), extensionRegistry);
-          return;
+          subBuilder = getFieldBuilder(field);
+          if (subBuilder != null) {
+            input.readMessage(subBuilder, extensionRegistry);
+            return;
+          } else {
+            subBuilder = newMessageFieldInstance(field, defaultInstance);
+            subBuilder.mergeFrom((Message) getField(field));
+          }
+        } else {
+          subBuilder = newMessageFieldInstance(field, defaultInstance);
         }
-        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readMessage(subBuilder, extensionRegistry);
         Object unused = setField(field, subBuilder.buildPartial());
       } else {
         Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
         input.readMessage(subBuilder, extensionRegistry);
@@ -584,15 +624,18 @@ class MessageReflection {
 
     @Override
     public MergeTarget newMergeTargetForField(
         Descriptors.FieldDescriptor field, Message defaultInstance) {
       Message.Builder subBuilder;
-      if (defaultInstance != null) {
-        subBuilder = defaultInstance.newBuilderForType();
-      } else {
-        subBuilder = builder.newBuilderForField(field);
+      if (!field.isRepeated() && hasField(field)) {
+        subBuilder = getFieldBuilder(field);
+        if (subBuilder != null) {
+          return new BuilderAdapter(subBuilder);
+        }
       }
+
+      subBuilder = newMessageFieldInstance(field, defaultInstance);
       if (!field.isRepeated()) {
         Message originalMessage = (Message) getField(field);
         if (originalMessage != null) {
           subBuilder.mergeFrom(originalMessage);
         }
@@ -624,11 +667,11 @@ class MessageReflection {
       return WireFormat.Utf8Validation.LOOSE;
     }
 
     @Override
     public Object finish() {
-      return builder.buildPartial();
+      return builder;
     }
   }
 
 
   static class ExtensionAdapter implements MergeTarget {

Vulnerable code:   * Make message-type extensions merge from wire-format instead of building up instances and merging afterwards. This has much better performance.
          input.readGroup(field.getNumber(), builder.getFieldBuilder(field), extensionRegistry);
          return;
        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
          input.readMessage(builder.getFieldBuilder(field), extensionRegistry);
          return;
        Message.Builder subBuilder = newMessageFieldInstance(field, defaultInstance);
      if (defaultInstance != null) {
        subBuilder = defaultInstance.newBuilderForType();
      } else {
        subBuilder = builder.newBuilderForField(field);
      return builder.buildPartial();
Fixed code:   * Make message-type extensions merge from wire-format instead of building up
    instances and merging afterwards. This has much better performance.
  * Fix TextFormat parser to build up recurring (but supposedly not repeated)
    sub-messages directly from text rather than building a new sub-message and
    merging the fully formed message into the existing field.
    private boolean hasNestedBuilders = true;
    private Message.Builder getFieldBuilder(Descriptors.FieldDescriptor field) {
      if (hasNestedBuilders) {
        try {
          return builder.getFieldBuilder(field);
        } catch (UnsupportedOperationException e) {
          hasNestedBuilders = false;
        }
      }
      return null;
    }

      if (!field.isRepeated() && value instanceof MessageLite.Builder) {
        if (value != getFieldBuilder(field)) {
          builder.setField(field, ((MessageLite.Builder) value).buildPartial());
        }
        return this;
      }
      if (value instanceof MessageLite.Builder) {
        value = ((MessageLite.Builder) value).buildPartial();
      }
      if (value instanceof MessageLite.Builder) {
        value = ((MessageLite.Builder) value).buildPartial();
      }
        Message.Builder subBuilder;
          subBuilder = getFieldBuilder(field);
          if (subBuilder != null) {
            input.readGroup(field.getNumber(), subBuilder, extensionRegistry);
            return;
          } else {
            subBuilder = newMessageFieldInstance(field, defaultInstance);
            subBuilder.mergeFrom((Message) getField(field));
          }
        } else {
          subBuilder = newMessageFieldInstance(field, defaultInstance);
        Message.Builder subBuilder;
          subBuilder = getFieldBuilder(field);
          if (subBuilder != null) {
            input.readMessage(subBuilder, extensionRegistry);
            return;
          } else {
            subBuilder = newMessageFieldInstance(field, defaultInstance);
            subBuilder.mergeFrom((Message) getField(field));
          }
        } else {
          subBuilder = newMessageFieldInstance(field, defaultInstance);
      if (!field.isRepeated() && hasField(field)) {
        subBuilder = getFieldBuilder(field);
        if (subBuilder != null) {
          return new BuilderAdapter(subBuilder);
        }

      subBuilder = newMessageFieldInstance(field, defaultInstance);
      return builder;