You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@bigtop.apache.org by se...@apache.org on 2022/04/25 13:07:17 UTC
[bigtop] branch master updated: BIGTOP-3671. Add a patch for CVE-2021-22569 to bigtop_toolchain. (#888)
This is an automated email from the ASF dual-hosted git repository.
sekikn pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/bigtop.git
The following commit(s) were added to refs/heads/master by this push:
new 02ca97be BIGTOP-3671. Add a patch for CVE-2021-22569 to bigtop_toolchain. (#888)
02ca97be is described below
commit 02ca97bec055bb64d42b134a0110e19ef2277db2
Author: Masatake Iwasaki <iw...@apache.org>
AuthorDate: Mon Apr 25 22:07:13 2022 +0900
BIGTOP-3671. Add a patch for CVE-2021-22569 to bigtop_toolchain. (#888)
---
...569-Improve-performance-of-parsing-unknow.patch | 430 +++++++++++++++++++++
bigtop_toolchain/manifests/protobuf.pp | 16 +-
2 files changed, 444 insertions(+), 2 deletions(-)
diff --git a/bigtop_toolchain/files/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch b/bigtop_toolchain/files/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch
new file mode 100644
index 00000000..c6192b4b
--- /dev/null
+++ b/bigtop_toolchain/files/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch
@@ -0,0 +1,430 @@
+From 0b10440b61f3328d7478b93fbf27e879a0dbac91 Mon Sep 17 00:00:00 2001
+From: Andrew Purtell <ap...@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
+
diff --git a/bigtop_toolchain/manifests/protobuf.pp b/bigtop_toolchain/manifests/protobuf.pp
index 6e43ce3e..47ea5a5b 100644
--- a/bigtop_toolchain/manifests/protobuf.pp
+++ b/bigtop_toolchain/manifests/protobuf.pp
@@ -26,11 +26,23 @@ class bigtop_toolchain::protobuf {
source => "puppet:///modules/bigtop_toolchain/0001-Backport-atomic-operations-with-support-of-arm64-and.patch"
}
+ file { "/usr/src/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch":
+ source => "puppet:///modules/bigtop_toolchain/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch"
+ }
+
exec { "download protobuf":
cwd => "/usr/src",
- command => "/usr/bin/wget $url/$protobuf8 && mkdir -p $protobuf8dir && /bin/tar -xvzf $protobuf8 -C $protobuf8dir --strip-components=1 && cd $protobuf8dir && /usr/bin/patch -p1 </usr/src/0001-Backport-atomic-operations-with-support-of-arm64-and.patch && curl -o config.guess 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' && cp config.guess gtest/build-aux/",
+ command => "/usr/bin/wget $url/$protobuf8 && \
+ mkdir -p $protobuf8dir && \
+ /bin/tar -xvzf $protobuf8 -C $protobuf8dir --strip-components=1 && \
+ cd $protobuf8dir && \
+ /usr/bin/patch -p1 </usr/src/0001-Backport-atomic-operations-with-support-of-arm64-and.patch && \
+ /usr/bin/patch -p1 </usr/src/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch && \
+ curl -o config.guess 'https://git.savannah.gnu.org/gitweb/?p=config.git;a=blob_plain;f=config.guess;hb=HEAD' && \
+ cp config.guess gtest/build-aux/",
creates => "/usr/src/$protobuf8dir",
- require => File["/usr/src/0001-Backport-atomic-operations-with-support-of-arm64-and.patch"]
+ require => [File["/usr/src/0001-Backport-atomic-operations-with-support-of-arm64-and.patch"],
+ File["/usr/src/0001-CVE-2021-22569-Improve-performance-of-parsing-unknow.patch"]]
}
exec { "install protobuf":