bigtop_toolchain/files/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch

From 0b10440b61f3328d7478b93fbf27e879a0dbac91 Mon Sep 17 00:00:00 2001
From: Andrew Purtell <apurtell@salesforce.com>
Date: Mon, 14 Feb 2022 15:00:03 -0800
Subject: [PATCH] CVE-2021-22569: Improve performance of parsing unknown fields
 in Java

An issue in protobuf-java allowed the interleaving of UnknownFieldSet fields
in such a way that would be processed out of order. A small malicious payload
can occupy the parser for several minutes by creating large numbers of short-
lived objects that cause frequent, repeated pauses.

Port fix from https://github.com/protocolbuffers/protobuf/pull/9371. It uses
a TreeMap to process fields in a well ordered way.

Reason: Security
Test Plan: Unit tests
---
 Makefile.am                                   |   1 +
 .../com/google/protobuf/UnknownFieldSet.java  | 179 +++++++++---------
 .../UnknownFieldSetPerformanceTest.java       |  76 ++++++++
 4 files changed, 171 insertions(+), 86 deletions(-)
 create mode 100644 java/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java

diff --git a/Makefile.am b/Makefile.am
index c9286132b..aedd021c3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -140,6 +140,7 @@ EXTRA_DIST =                                                                 \
   java/src/test/java/com/google/protobuf/TestUtil.java                       \
   java/src/test/java/com/google/protobuf/TextFormatTest.java                 \
   java/src/test/java/com/google/protobuf/UnknownFieldSetTest.java            \
+  java/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java \
   java/src/test/java/com/google/protobuf/UnmodifiableLazyStringListTest.java \
   java/src/test/java/com/google/protobuf/WireFormatTest.java                 \
   java/src/test/java/com/google/protobuf/multiple_files_test.proto           \
diff --git a/java/src/main/java/com/google/protobuf/UnknownFieldSet.java b/java/src/main/java/com/google/protobuf/UnknownFieldSet.java
index 45e2e6e40..f02377cf0 100644
--- a/java/src/main/java/com/google/protobuf/UnknownFieldSet.java
+++ b/java/src/main/java/com/google/protobuf/UnknownFieldSet.java
@@ -57,7 +57,6 @@ import java.util.TreeMap;
  * @author kenton@google.com Kenton Varda
  */
 public final class UnknownFieldSet implements MessageLite {
-  private UnknownFieldSet() {}
 
   /** Create a new {@link Builder}. */
   public static Builder newBuilder() {
@@ -80,16 +79,20 @@ public final class UnknownFieldSet implements MessageLite {
     return defaultInstance;
   }
   private static final UnknownFieldSet defaultInstance =
-    new UnknownFieldSet(Collections.<Integer, Field>emptyMap());
+    new UnknownFieldSet(new TreeMap<Integer, Field>());
+
+  private UnknownFieldSet() {
+    fields = new TreeMap<Integer, Field>();
+  }
 
   /**
    * Construct an {@code UnknownFieldSet} around the given map.  The map is
    * expected to be immutable.
    */
-  private UnknownFieldSet(final Map<Integer, Field> fields) {
+  private UnknownFieldSet(final TreeMap<Integer, Field> fields) {
     this.fields = fields;
   }
-  private Map<Integer, Field> fields;
+  private final TreeMap<Integer, Field> fields;
 
   @Override
   public boolean equals(final Object other) {
@@ -196,8 +199,10 @@ public final class UnknownFieldSet implements MessageLite {
   /** Get the number of bytes required to encode this set. */
   public int getSerializedSize() {
     int result = 0;
-    for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
-      result += entry.getValue().getSerializedSize(entry.getKey());
+    if (!fields.isEmpty()) {
+      for (final Map.Entry<Integer, Field> entry : fields.entrySet()) {
+        result += entry.getValue().getSerializedSize(entry.getKey());
+      }
     }
     return result;
   }
@@ -281,62 +286,44 @@ public final class UnknownFieldSet implements MessageLite {
     // This constructor should never be called directly (except from 'create').
     private Builder() {}
 
-    private Map<Integer, Field> fields;
-
-    // Optimization:  We keep around a builder for the last field that was
-    //   modified so that we can efficiently add to it multiple times in a
-    //   row (important when parsing an unknown repeated field).
-    private int lastFieldNumber;
-    private Field.Builder lastField;
+    private TreeMap<Integer, Field.Builder> fieldBuilders =
+        new TreeMap<Integer, Field.Builder>();
 
     private static Builder create() {
-      Builder builder = new Builder();
-      builder.reinitialize();
-      return builder;
+      return new Builder();
     }
 
     /**
      * Get a field builder for the given field number which includes any
      * values that already exist.
      */
-    private Field.Builder getFieldBuilder(final int number) {
-      if (lastField != null) {
-        if (number == lastFieldNumber) {
-          return lastField;
-        }
-        // Note:  addField() will reset lastField and lastFieldNumber.
-        addField(lastFieldNumber, lastField.build());
-      }
+    private Field.Builder getFieldBuilder(int number) {
       if (number == 0) {
         return null;
       } else {
-        final Field existing = fields.get(number);
-        lastFieldNumber = number;
-        lastField = Field.newBuilder();
-        if (existing != null) {
-          lastField.mergeFrom(existing);
+        Field.Builder builder = fieldBuilders.get(number);
+        if (builder == null) {
+          builder = Field.newBuilder();
+          fieldBuilders.put(number, builder);
         }
-        return lastField;
+        return builder;
       }
     }
 
     /**
      * Build the {@link UnknownFieldSet} and return it.
-     *
-     * <p>Once {@code build()} has been called, the {@code Builder} will no
-     * longer be usable.  Calling any method after {@code build()} will result
-     * in undefined behavior and can cause a {@code NullPointerException} to be
-     * thrown.
      */
     public UnknownFieldSet build() {
-      getFieldBuilder(0);  // Force lastField to be built.
-      final UnknownFieldSet result;
-      if (fields.isEmpty()) {
+      UnknownFieldSet result;
+      if (fieldBuilders.isEmpty()) {
         result = getDefaultInstance();
       } else {
-        result = new UnknownFieldSet(Collections.unmodifiableMap(fields));
+        TreeMap<Integer, Field> fields = new TreeMap<Integer, Field>();
+        for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+          fields.put(entry.getKey(), entry.getValue().build());
+        }
+        result = new UnknownFieldSet(fields);
       }
-      fields = null;
       return result;
     }
 
@@ -347,24 +334,22 @@ public final class UnknownFieldSet implements MessageLite {
 
     @Override
     public Builder clone() {
-      getFieldBuilder(0);  // Force lastField to be built.
-      return UnknownFieldSet.newBuilder().mergeFrom(
-          new UnknownFieldSet(fields));
+      Builder clone = UnknownFieldSet.newBuilder();
+      for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+        Integer key = entry.getKey();
+        Field.Builder value = entry.getValue();
+        clone.fieldBuilders.put(key, value.clone());
+      }
+      return clone;
     }
 
     public UnknownFieldSet getDefaultInstanceForType() {
       return UnknownFieldSet.getDefaultInstance();
     }
 
-    private void reinitialize() {
-      fields = Collections.emptyMap();
-      lastFieldNumber = 0;
-      lastField = null;
-    }
-
     /** Reset the builder to an empty set. */
     public Builder clear() {
-      reinitialize();
+      fieldBuilders = new TreeMap<Integer, Field.Builder>();
       return this;
     }
 
@@ -415,11 +400,8 @@ public final class UnknownFieldSet implements MessageLite {
     }
 
     /** Check if the given field number is present in the set. */
-    public boolean hasField(final int number) {
-      if (number == 0) {
-        throw new IllegalArgumentException("Zero is not a valid field number.");
-      }
-      return number == lastFieldNumber || fields.containsKey(number);
+    public boolean hasField(int number) {
+      return fieldBuilders.containsKey(number);
     }
 
     /**
@@ -430,15 +412,7 @@ public final class UnknownFieldSet implements MessageLite {
       if (number == 0) {
         throw new IllegalArgumentException("Zero is not a valid field number.");
       }
-      if (lastField != null && lastFieldNumber == number) {
-        // Discard this.
-        lastField = null;
-        lastFieldNumber = 0;
-      }
-      if (fields.isEmpty()) {
-        fields = new TreeMap<Integer,Field>();
-      }
-      fields.put(number, field);
+      fieldBuilders.put(number, Field.newBuilder(field));
       return this;
     }
 
@@ -447,7 +421,10 @@ public final class UnknownFieldSet implements MessageLite {
      * fields are added, the changes may or may not be reflected in this map.
      */
     public Map<Integer, Field> asMap() {
-      getFieldBuilder(0);  // Force lastField to be built.
+      TreeMap<Integer, Field> fields = new TreeMap<Integer, Field>();
+      for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
+        fields.put(entry.getKey(), entry.getValue().build());
+      }
       return Collections.unmodifiableMap(fields);
     }
 
@@ -809,54 +786,84 @@ public final class UnknownFieldSet implements MessageLite {
      * <p>Use {@link Field#newBuilder()} to construct a {@code Builder}.
      */
     public static final class Builder {
-      // This constructor should never be called directly (except from 'create').
-      private Builder() {}
+      // This constructor should only be called directly from 'create' and 'clone'.
+      private Builder() {
+        result = new Field();
+      }
 
       private static Builder create() {
         Builder builder = new Builder();
-        builder.result = new Field();
         return builder;
       }
 
       private Field result;
 
+      @Override
+      public Builder clone() {
+        Field copy = new Field();
+        if (result.varint == null) {
+          copy.varint = null;
+        } else {
+          copy.varint = new ArrayList<Long>(result.varint);
+        }
+        if (result.fixed32 == null) {
+          copy.fixed32 = null;
+        } else {
+          copy.fixed32 = new ArrayList<Integer>(result.fixed32);
+        }
+        if (result.fixed64 == null) {
+          copy.fixed64 = null;
+        } else {
+          copy.fixed64 = new ArrayList<Long>(result.fixed64);
+        }
+        if (result.lengthDelimited == null) {
+          copy.lengthDelimited = null;
+        } else {
+          copy.lengthDelimited = new ArrayList<ByteString>(result.lengthDelimited);
+        }
+        if (result.group == null) {
+          copy.group = null;
+        } else {
+          copy.group = new ArrayList<UnknownFieldSet>(result.group);
+        }
+
+        Builder clone = new Builder();
+        clone.result = copy;
+        return clone;
+      }
+
       /**
-       * Build the field.  After {@code build()} has been called, the
-       * {@code Builder} is no longer usable.  Calling any other method will
-       * result in undefined behavior and can cause a
-       * {@code NullPointerException} to be thrown.
+       * Build the field.
        */
       public Field build() {
+        Field built = new Field();
         if (result.varint == null) {
-          result.varint = Collections.emptyList();
+          built.varint = Collections.emptyList();
         } else {
-          result.varint = Collections.unmodifiableList(result.varint);
+          built.varint = Collections.unmodifiableList(result.varint);
         }
         if (result.fixed32 == null) {
-          result.fixed32 = Collections.emptyList();
+          built.fixed32 = Collections.emptyList();
         } else {
-          result.fixed32 = Collections.unmodifiableList(result.fixed32);
+          built.fixed32 = Collections.unmodifiableList(result.fixed32);
         }
         if (result.fixed64 == null) {
-          result.fixed64 = Collections.emptyList();
+          built.fixed64 = Collections.emptyList();
         } else {
-          result.fixed64 = Collections.unmodifiableList(result.fixed64);
+          built.fixed64 = Collections.unmodifiableList(result.fixed64);
         }
         if (result.lengthDelimited == null) {
-          result.lengthDelimited = Collections.emptyList();
+          built.lengthDelimited = Collections.emptyList();
         } else {
-          result.lengthDelimited =
+          built.lengthDelimited =
             Collections.unmodifiableList(result.lengthDelimited);
         }
         if (result.group == null) {
-          result.group = Collections.emptyList();
+          built.group = Collections.emptyList();
         } else {
-          result.group = Collections.unmodifiableList(result.group);
+          built.group = Collections.unmodifiableList(result.group);
         }
-
-        final Field returnMe = result;
-        result = null;
-        return returnMe;
+        return built;
       }
 
       /** Discard the field's contents. */
diff --git a/java/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java b/java/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
new file mode 100644
index 000000000..9b913b50f
--- /dev/null
+++ b/java/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
@@ -0,0 +1,76 @@
+// Protocol Buffers - Google's data interchange format
+// Copyright 2008 Google Inc.  All rights reserved.
+// https://developers.google.com/protocol-buffers/
+//
+// Redistribution and use in source and binary forms, with or without
+// modification, are permitted provided that the following conditions are
+// met:
+//
+//     * Redistributions of source code must retain the above copyright
+// notice, this list of conditions and the following disclaimer.
+//     * Redistributions in binary form must reproduce the above
+// copyright notice, this list of conditions and the following disclaimer
+// in the documentation and/or other materials provided with the
+// distribution.
+//     * Neither the name of Google Inc. nor the names of its
+// contributors may be used to endorse or promote products derived from
+// this software without specific prior written permission.
+//
+// THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS
+// "AS IS" AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT
+// LIMITED TO, THE IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR
+// A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT
+// OWNER OR CONTRIBUTORS BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
+// SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT
+// LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE,
+// DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY
+// THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+// (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
+// OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+
+package com.google.protobuf;
+
+import java.io.ByteArrayInputStream;
+import java.io.IOException;
+import java.io.InputStream;
+
+import org.junit.Assert;
+
+import junit.framework.TestCase;
+
+public final class UnknownFieldSetPerformanceTest extends TestCase {
+
+  private static byte[] generateBytes(int length) {
+    Assert.assertTrue((length % 4) == 0);
+    byte[] input = new byte[length];
+    for (int i = 0; i < length; i += 4) {
+        input[i] =     (byte) 0x08; // field 1, wiretype 0
+        input[i + 1] = (byte) 0x08; // field 1, payload 8
+        input[i + 2] = (byte) 0x20; // field 4, wiretype 0
+        input[i + 3] = (byte) 0x20; // field 4, payload 32
+    }
+    return input;
+  }
+
+  // This is a performance test. Failure here is a timeout.
+  public void testAlternatingFieldNumbers() throws IOException {
+    byte[] input = generateBytes(800000);
+    InputStream in = new ByteArrayInputStream(input);
+    UnknownFieldSet.Builder builder = UnknownFieldSet.newBuilder();
+    CodedInputStream codedInput = CodedInputStream.newInstance(in);
+    builder.mergeFrom(codedInput);
+  }
+
+  // This is a performance test. Failure here is a timeout.
+  public void testAddField() {
+    UnknownFieldSet.Builder builder = UnknownFieldSet.newBuilder();
+    for (int i = 1; i <= 100000; i++) {
+      UnknownFieldSet.Field field = UnknownFieldSet.Field.newBuilder().addFixed32(i).build();
+      builder.addField(i, field);
+    }
+    UnknownFieldSet fieldSet = builder.build();
+    assertTrue(fieldSet.getField(100000).getFixed32List().get(0) == 100000);
+  }
+
+}
+
-- 
2.33.0