Bug#1034510: bullseye-pu: package protobuf/3.12.4-1+deb11u1
Package: release.debian.org
Severity: normal
Tags: bullseye
User: release.debian.org@packages.debian.org
Usertags: pu
X-Debbugs-Cc: protobuf@packages.debian.org, gcs@debian.org, debian-security@lists.debian.org
Control: affects -1 + src:protobuf
[ Reason ]
This update aims to fix three vulnerabilities in the protobuf package:
* Fix CVE-2021-22569 (DoS in Java)
* Fix CVE-2021-22570 (NULL pointer dereference)
* Fix CVE-2022-1941 (memory DoS)
[ Impact ]
Ideally, a user wouldn't notice anything changed.
[ Tests ]
Testing was accidentally disabled in bullseye. See #1033989. This upload
re-enables testing. The patch for CVE-2022-1941 extends the test suite.
[ Risks ]
All of the patches deviate significantly from their respective upstream
commits. In case of CVE-2021-22569 and CVE-2022-1941, significant
porting was required. In case of CVE-2021-22570, the relevant change was
buried in a large commit. There definitely is a risk involved here. I do
appreciate a review of these patches.
[ Checklist ]
[x] *all* changes are documented in the d/changelog
[x] I reviewed all changes and I approve them
[x] attach debdiff against the package in (old)stable
[x] the issue is verified as fixed in unstable
[ Changes ]
Beyond fixing the vulnerabilities (see Reason section), this upload also
re-enables running tests during build.
Helmut
diff --minimal -Nru protobuf-3.12.4/debian/changelog protobuf-3.12.4/debian/changelog
--- protobuf-3.12.4/debian/changelog 2021-01-16 23:12:54.000000000 +0100
+++ protobuf-3.12.4/debian/changelog 2023-04-04 11:41:41.000000000 +0200
@@ -1,3 +1,13 @@
+protobuf (3.12.4-1+deb11u1) bullseye; urgency=medium
+
+ * Non-maintainer upload.
+ * Reenable test suite (Closes: #1033989)
+ * Fix CVE-2021-22569 (DoS in Java)
+ * Fix CVE-2021-22570 (NULL pointer dereference)
+ * Fix CVE-2022-1941 (memory DoS)
+
+ -- Helmut Grohne <helmut@subdivi.de> Tue, 04 Apr 2023 11:41:41 +0200
+
protobuf (3.12.4-1) unstable; urgency=medium
* New upstream release.
diff --minimal -Nru protobuf-3.12.4/debian/elpa-test protobuf-3.12.4/debian/elpa-test
--- protobuf-3.12.4/debian/elpa-test 1970-01-01 01:00:00.000000000 +0100
+++ protobuf-3.12.4/debian/elpa-test 2023-04-04 11:41:41.000000000 +0200
@@ -0,0 +1 @@
+disable=please_run_dh_auto_test
diff --minimal -Nru protobuf-3.12.4/debian/patches/CVE-2021-22569.patch protobuf-3.12.4/debian/patches/CVE-2021-22569.patch
--- protobuf-3.12.4/debian/patches/CVE-2021-22569.patch 1970-01-01 01:00:00.000000000 +0100
+++ protobuf-3.12.4/debian/patches/CVE-2021-22569.patch 2023-04-04 11:41:41.000000000 +0200
@@ -0,0 +1,482 @@
+From 9638a5e5315bf73f5e7148c16181676372321892 Mon Sep 17 00:00:00 2001
+From: Adam Cozzette <acozzette@google.com>
+Date: Wed, 5 Jan 2022 08:50:29 -0800
+Subject: [PATCH] Improve performance of parsing unknown fields in Java (#9371)
+
+Credit should go to @elharo for most of these Java changes--I am just
+cherry-picking them from our internal codebase. The one thing I did
+change was to give the UTF-8 validation tests their own Bazel test
+target. This makes it possible to give the other tests a shorter
+timeout, which is important for UnknownFieldSetPerformanceTest in
+particular.
+---
+ Makefile.am | 1 +
+ java/core/BUILD | 24 +-
+ .../com/google/protobuf/UnknownFieldSet.java | 427 +++++++++---------
+ .../UnknownFieldSetPerformanceTest.java | 78 ++++
+ .../google/protobuf/UnknownFieldSetTest.java | 182 +++++++-
+ java/lite/pom.xml | 1 +
+ 6 files changed, 499 insertions(+), 214 deletions(-)
+ create mode 100644 java/core/src/test/java/com/google/protobuf/UnknownFieldSetPerformanceTest.java
+
+Backport:
+ * Drop bazel BUILD file changes as Debian builds using ant
+ * Drop test cases as Debian does not run them
+ * Drop unnecessary de-finalization
+ * Drop unnecessary diamonds conversion
+
+diff --git a/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java b/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
+index ba2f9df08..5c482d62d 100644
+--- a/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
++++ b/java/core/src/main/java/com/google/protobuf/UnknownFieldSet.java
+@@ -43,13 +43,13 @@ import java.util.Map;
+ import java.util.TreeMap;
+
+ /**
+- * {@code UnknownFieldSet} is used to keep track of fields which were seen when parsing a protocol
++ * {@code UnknownFieldSet} keeps track of fields which were seen when parsing a protocol
+ * message but whose field numbers or types are unrecognized. This most frequently occurs when new
+ * fields are added to a message type and then messages containing those fields are read by old
+ * software that was compiled before the new types were added.
+ *
+ * <p>Every {@link Message} contains an {@code UnknownFieldSet} (and every {@link Message.Builder}
+- * contains an {@link Builder}).
++ * contains a {@link Builder}).
+ *
+ * <p>Most users will never need to use this class.
+ *
+@@ -57,9 +57,13 @@ import java.util.TreeMap;
+ */
+ public final class UnknownFieldSet implements MessageLite {
+
+- private UnknownFieldSet() {
+- fields = null;
+- fieldsDescending = null;
++ private final TreeMap<Integer, Field> fields;
++
++ /**
++ * Construct an {@code UnknownFieldSet} around the given map.
++ */
++ UnknownFieldSet(TreeMap<Integer, Field> fields) {
++ this.fields = fields;
+ }
+
+ /** Create a new {@link Builder}. */
+@@ -83,21 +87,7 @@ public final class UnknownFieldSet implements MessageLite {
+ }
+
+ private static final UnknownFieldSet defaultInstance =
+- new UnknownFieldSet(
+- Collections.<Integer, Field>emptyMap(), Collections.<Integer, Field>emptyMap());
+-
+- /**
+- * Construct an {@code UnknownFieldSet} around the given map. The map is expected to be immutable.
+- */
+- UnknownFieldSet(final Map<Integer, Field> fields, final Map<Integer, Field> fieldsDescending) {
+- this.fields = fields;
+- this.fieldsDescending = fieldsDescending;
+- }
+-
+- private final Map<Integer, Field> fields;
+-
+- /** A copy of {@link #fields} who's iterator order is reversed. */
+- private final Map<Integer, Field> fieldsDescending;
++ new UnknownFieldSet(new TreeMap<Integer, Field>());
+
+
+ @Override
+@@ -110,12 +100,16 @@ public final class UnknownFieldSet implements MessageLite {
+
+ @Override
+ public int hashCode() {
++ if (fields.isEmpty()) { // avoid allocation of iterator.
++ // This optimization may not be helpful but it is needed for the allocation tests to pass.
++ return 0;
++ }
+ return fields.hashCode();
+ }
+
+ /** Get a map of fields in the set by number. */
+ public Map<Integer, Field> asMap() {
+- return fields;
++ return (Map<Integer, Field>) fields.clone();
+ }
+
+ /** Check if the given field number is present in the set. */
+@@ -195,7 +189,7 @@ public final class UnknownFieldSet implements MessageLite {
+ @Override
+ public void writeDelimitedTo(OutputStream output) throws IOException {
+ final CodedOutputStream codedOutput = CodedOutputStream.newInstance(output);
+- codedOutput.writeRawVarint32(getSerializedSize());
++ codedOutput.writeUInt32NoTag(getSerializedSize());
+ writeTo(codedOutput);
+ codedOutput.flush();
+ }
+@@ -204,8 +198,10 @@ public final class UnknownFieldSet implements MessageLite {
+ @Override
+ 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 (Map.Entry<Integer, Field> entry : fields.entrySet()) {
++ result += entry.getValue().getSerializedSize(entry.getKey());
++ }
+ }
+ return result;
+ }
+@@ -221,7 +217,7 @@ public final class UnknownFieldSet implements MessageLite {
+ void writeTo(Writer writer) throws IOException {
+ if (writer.fieldOrder() == Writer.FieldOrder.DESCENDING) {
+ // Write fields in descending order.
+- for (Map.Entry<Integer, Field> entry : fieldsDescending.entrySet()) {
++ for (Map.Entry<Integer, Field> entry : fields.descendingMap().entrySet()) {
+ entry.getValue().writeTo(entry.getKey(), writer);
+ }
+ } else {
+@@ -236,7 +232,7 @@ public final class UnknownFieldSet implements MessageLite {
+ void writeAsMessageSetTo(final Writer writer) throws IOException {
+ if (writer.fieldOrder() == Writer.FieldOrder.DESCENDING) {
+ // Write fields in descending order.
+- for (final Map.Entry<Integer, Field> entry : fieldsDescending.entrySet()) {
++ for (final Map.Entry<Integer, Field> entry : fields.descendingMap().entrySet()) {
+ entry.getValue().writeAsMessageSetExtensionTo(entry.getKey(), writer);
+ }
+ } else {
+@@ -309,64 +305,43 @@ 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<>();
+
+ 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());
+- }
+ 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.
+ */
+ @Override
+ public UnknownFieldSet build() {
+- getFieldBuilder(0); // Force lastField to be built.
+ final UnknownFieldSet result;
+- if (fields.isEmpty()) {
++ if (fieldBuilders.isEmpty()) {
+ result = getDefaultInstance();
+ } else {
+- Map<Integer, Field> descendingFields = null;
+- descendingFields =
+- Collections.unmodifiableMap(((TreeMap<Integer, Field>) fields).descendingMap());
+- result = new UnknownFieldSet(Collections.unmodifiableMap(fields), descendingFields);
++ TreeMap<Integer, Field> fields = new TreeMap<>();
++ for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
++ fields.put(entry.getKey(), entry.getValue().build());
++ }
++ result = new UnknownFieldSet(fields);
+ }
+- fields = null;
+ return result;
+ }
+
+@@ -378,11 +353,13 @@ public final class UnknownFieldSet implements MessageLite {
+
+ @Override
+ public Builder clone() {
+- getFieldBuilder(0); // Force lastField to be built.
+- Map<Integer, Field> descendingFields = null;
+- descendingFields =
+- Collections.unmodifiableMap(((TreeMap<Integer, Field>) fields).descendingMap());
+- return UnknownFieldSet.newBuilder().mergeFrom(new UnknownFieldSet(fields, descendingFields));
++ 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;
+ }
+
+ @Override
+@@ -390,31 +367,24 @@ public final class UnknownFieldSet implements MessageLite {
+ return UnknownFieldSet.getDefaultInstance();
+ }
+
+- private void reinitialize() {
+- fields = Collections.emptyMap();
+- lastFieldNumber = 0;
+- lastField = null;
+- }
+-
+ /** Reset the builder to an empty set. */
+ @Override
+ public Builder clear() {
+- reinitialize();
++ fieldBuilders = new TreeMap<>();
+ return this;
+ }
+
+- /** Clear fields from the set with a given field number. */
++ /**
++ * Clear fields from the set with a given field number.
++ *
++ * @throws IllegalArgumentException if number is not positive
++ */
+ public Builder clearField(final int number) {
+- 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 (number <= 0) {
++ throw new IllegalArgumentException(number + " is not a valid field number.");
+ }
+- if (fields.containsKey(number)) {
+- fields.remove(number);
++ if (fieldBuilders.containsKey(number)) {
++ fieldBuilders.remove(number);
+ }
+ return this;
+ }
+@@ -435,10 +405,12 @@ public final class UnknownFieldSet implements MessageLite {
+ /**
+ * Add a field to the {@code UnknownFieldSet}. If a field with the same number already exists,
+ * the two are merged.
++ *
++ * @throws IllegalArgumentException if number is not positive
+ */
+ public Builder mergeField(final int number, final Field field) {
+- if (number == 0) {
+- throw new IllegalArgumentException("Zero is not a valid field number.");
++ if (number <= 0) {
++ throw new IllegalArgumentException(number + " is not a valid field number.");
+ }
+ if (hasField(number)) {
+ getFieldBuilder(number).mergeFrom(field);
+@@ -454,10 +426,12 @@ public final class UnknownFieldSet implements MessageLite {
+ /**
+ * Convenience method for merging a new field containing a single varint value. This is used in
+ * particular when an unknown enum value is encountered.
++ *
++ * @throws IllegalArgumentException if number is not positive
+ */
+ public Builder mergeVarintField(final int number, final int value) {
+- if (number == 0) {
+- throw new IllegalArgumentException("Zero is not a valid field number.");
++ if (number <= 0) {
++ throw new IllegalArgumentException(number + " is not a valid field number.");
+ }
+ getFieldBuilder(number).addVarint(value);
+ return this;
+@@ -467,10 +441,12 @@ public final class UnknownFieldSet implements MessageLite {
+ * Convenience method for merging a length-delimited field.
+ *
+ * <p>For use by generated code only.
++ *
++ * @throws IllegalArgumentException if number is not positive
+ */
+ public Builder mergeLengthDelimitedField(final int number, final ByteString value) {
+- if (number == 0) {
+- throw new IllegalArgumentException("Zero is not a valid field number.");
++ if (number <= 0) {
++ throw new IllegalArgumentException(number + " is not a valid field number.");
+ }
+ getFieldBuilder(number).addLengthDelimited(value);
+ return this;
+@@ -478,29 +454,20 @@ 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);
++ return fieldBuilders.containsKey(number);
+ }
+
+ /**
+ * Add a field to the {@code UnknownFieldSet}. If a field with the same number already exists,
+ * it is removed.
++ *
++ * @throws IllegalArgumentException if number is not positive
+ */
+ public Builder addField(final int number, final Field field) {
+- 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 (number <= 0) {
++ throw new IllegalArgumentException(number + " is not a valid field number.");
+ }
+- if (fields.isEmpty()) {
+- fields = new TreeMap<Integer, Field>();
+- }
+- fields.put(number, field);
++ fieldBuilders.put(number, Field.newBuilder(field));
+ return this;
+ }
+
+@@ -509,7 +476,10 @@ public final class UnknownFieldSet implements MessageLite {
+ * 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<>();
++ for (Map.Entry<Integer, Field.Builder> entry : fieldBuilders.entrySet()) {
++ fields.put(entry.getKey(), entry.getValue().build());
++ }
+ return Collections.unmodifiableMap(fields);
+ }
+
+@@ -912,52 +882,85 @@ 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<>(result.varint);
++ }
++ if (result.fixed32 == null) {
++ copy.fixed32 = null;
++ } else {
++ copy.fixed32 = new ArrayList<>(result.fixed32);
++ }
++ if (result.fixed64 == null) {
++ copy.fixed64 = null;
++ } else {
++ copy.fixed64 = new ArrayList<>(result.fixed64);
++ }
++ if (result.lengthDelimited == null) {
++ copy.lengthDelimited = null;
++ } else {
++ copy.lengthDelimited = new ArrayList<>(result.lengthDelimited);
++ }
++ if (result.group == null) {
++ copy.group = null;
++ } else {
++ copy.group = new ArrayList<>(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(new ArrayList<>(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(new ArrayList<>(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(new ArrayList<>(result.fixed64));
+ }
+ if (result.lengthDelimited == null) {
+- result.lengthDelimited = Collections.emptyList();
++ built.lengthDelimited = Collections.emptyList();
+ } else {
+- result.lengthDelimited = Collections.unmodifiableList(result.lengthDelimited);
++ built.lengthDelimited = Collections.unmodifiableList(
++ new ArrayList<>(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(new ArrayList<>(result.group));
+ }
+
+- final Field returnMe = result;
+- result = null;
+- return returnMe;
++ return built;
+ }
+
+ /** Discard the field's contents. */
diff --minimal -Nru protobuf-3.12.4/debian/patches/CVE-2021-22570.patch protobuf-3.12.4/debian/patches/CVE-2021-22570.patch
--- protobuf-3.12.4/debian/patches/CVE-2021-22570.patch 1970-01-01 01:00:00.000000000 +0100
+++ protobuf-3.12.4/debian/patches/CVE-2021-22570.patch 2023-04-04 11:41:41.000000000 +0200
@@ -0,0 +1,53 @@
+commit a00125024e9231d76746bd394fef8876f5cc15e2
+Merge: 5c028d6cf 468bc193e
+Author: Deanna Garcia <deannagarcia@google.com>
+Date: Fri Jan 22 00:24:30 2021 +0000
+
+ Sync from Piper @353127564
+
+ PROTOBUF_SYNC_PIPER
+
+Backport:
+ * Reduce to the actually relevant changes introduced in the actual merge.
+
+diff --cc src/google/protobuf/descriptor.cc
+index 7af37c57f,7af37c57f..03c4e2b51
+--- a/src/google/protobuf/descriptor.cc
++++ b/src/google/protobuf/descriptor.cc
+@@ -4019,6 +4023,11 @@ bool DescriptorBuilder::AddSymbol(cons
+ // Use its file as the parent instead.
+ if (parent == nullptr) parent = file_;
+
++ if (full_name.find('\0') != std::string::npos) {
++ AddError(full_name, proto, DescriptorPool::ErrorCollector::NAME,
++ "\"" + full_name + "\" contains null character.");
++ return false;
++ }
+ if (tables_->AddSymbol(full_name, symbol)) {
+ if (!file_tables_->AddAliasUnderParent(parent, name, symbol)) {
+ // This is only possible if there was already an error adding something of
+@@ -4059,6 +4068,11 @@
+ void DescriptorBuilder::AddPackage(const std::string& name,
+ const Message& proto,
+ const FileDescriptor* file) {
++ if (name.find('\0') != std::string::npos) {
++ AddError(name, proto, DescriptorPool::ErrorCollector::NAME,
++ "\"" + name + "\" contains null character.");
++ return;
++ }
+ if (tables_->AddSymbol(name, Symbol(file))) {
+ // Success. Also add parent package, if any.
+ std::string::size_type dot_pos = name.find_last_of('.');
+@@ -4372,6 +4386,12 @@ FileDescriptor* DescriptorBuilder::Buil
+ }
+ result->pool_ = pool_;
+
++ if (result->name().find('\0') != std::string::npos) {
++ AddError(result->name(), proto, DescriptorPool::ErrorCollector::NAME,
++ "\"" + result->name() + "\" contains null character.");
++ return nullptr;
++ }
++
+ // Add to tables.
+ if (!tables_->AddFile(result)) {
+ AddError(proto.name(), proto, DescriptorPool::ErrorCollector::OTHER,
diff --minimal -Nru protobuf-3.12.4/debian/patches/CVE-2022-1941.patch protobuf-3.12.4/debian/patches/CVE-2022-1941.patch
--- protobuf-3.12.4/debian/patches/CVE-2022-1941.patch 1970-01-01 01:00:00.000000000 +0100
+++ protobuf-3.12.4/debian/patches/CVE-2022-1941.patch 2023-04-04 11:41:41.000000000 +0200
@@ -0,0 +1,364 @@
+From 7764c864bd5acdf60230a7b8fd29816170d0d04e Mon Sep 17 00:00:00 2001
+From: Mike Kruskal <mkruskal@google.com>
+Date: Mon, 12 Sep 2022 14:39:23 -0700
+Subject: [PATCH] Sync from Piper @473817856
+
+PROTOBUF_SYNC_PIPER
+---
+ CHANGES.txt | 2 +
+ src/google/protobuf/extension_set_inl.h | 27 +++--
+ src/google/protobuf/wire_format.cc | 26 +++--
+ src/google/protobuf/wire_format_lite.h | 27 +++--
+ src/google/protobuf/wire_format_unittest.inc | 104 +++++++++++++++++--
+ 5 files changed, 151 insertions(+), 35 deletions(-)
+
+Backport:
+ * Handle rename of testcase file.
+ * Drop CHANGES.txt, was:
+ * Save code space by avoiding inlining of large-in-aggregate code-space MessageLite::~MessageLite destructor.
+ * Undefine the macro `linux` when compiling protobuf
+ * Drop payload_read removal, which doesn't exist yet
+ * Drop _t suffix from arithmetic types
+ * Adapt casing in test code
+ * Add missing #include dynamic_message.h
+
+diff --git a/src/google/protobuf/extension_set_inl.h b/src/google/protobuf/extension_set_inl.h
+index f98065c60..825645540 100644
+--- a/src/google/protobuf/extension_set_inl.h
++++ b/src/google/protobuf/extension_set_inl.h
+@@ -207,14 +207,20 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
+ internal::InternalMetadata* metadata, internal::ParseContext* ctx) {
+ std::string payload;
+ uint32 type_id = 0;
++ enum class State { kNoTag, kHasType, kHasPayload, kDone };
++ State state = State::kNoTag;
++
+ while (!ctx->Done(&ptr)) {
+ uint32 tag = static_cast<uint8>(*ptr++);
+ if (tag == WireFormatLite::kMessageSetTypeIdTag) {
+ uint64 tmp;
+ ptr = ParseBigVarint(ptr, &tmp);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+- type_id = tmp;
+- if (!payload.empty()) {
++ if (state == State::kNoTag) {
++ type_id = tmp;
++ state = State::kHasType;
++ } else if (state == State::kHasPayload) {
++ type_id = tmp;
+ ExtensionInfo extension;
+ bool was_packed_on_wire;
+ if (!FindExtension(2, type_id, containing_type, ctx, &extension,
+@@ -240,19 +245,24 @@ const char* ExtensionSet::ParseMessageSetItemTmpl(
+ GOOGLE_PROTOBUF_PARSER_ASSERT(value->_InternalParse(p, &tmp_ctx) &&
+ tmp_ctx.EndedAtLimit());
+ }
+- type_id = 0;
++ state = State::kDone;
+ }
+ } else if (tag == WireFormatLite::kMessageSetMessageTag) {
+- if (type_id != 0) {
++ if (state == State::kHasType) {
+ ptr = ParseFieldMaybeLazily(static_cast<uint64>(type_id) * 8 + 2, ptr,
+ containing_type, metadata, ctx);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr != nullptr);
+- type_id = 0;
++ state = State::kDone;
+ } else {
++ std::string tmp;
+ int32 size = ReadSize(&ptr);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+- ptr = ctx->ReadString(ptr, size, &payload);
++ ptr = ctx->ReadString(ptr, size, &tmp);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
++ if (state == State::kNoTag) {
++ payload = std::move(tmp);
++ state = State::kHasPayload;
++ }
+ }
+ } else {
+ ptr = ReadTag(ptr - 1, &tag);
+diff --git a/src/google/protobuf/wire_format.cc b/src/google/protobuf/wire_format.cc
+index e42d44a9f..d9d291305 100644
+--- a/src/google/protobuf/wire_format.cc
++++ b/src/google/protobuf/wire_format.cc
+@@ -659,6 +659,9 @@ struct WireFormat::MessageSetParser {
+ const char* _InternalParse(const char* ptr, internal::ParseContext* ctx) {
+ // Parse a MessageSetItem
+ auto metadata = reflection->MutableInternalMetadata(msg);
++ enum class State { kNoTag, kHasType, kHasPayload, kDone };
++ State state = State::kNoTag;
++
+ std::string payload;
+ uint32 type_id = 0;
+ while (!ctx->Done(&ptr)) {
+@@ -669,8 +671,11 @@ struct WireFormat::MessageSetParser {
+ uint64 tmp;
+ ptr = ParseBigVarint(ptr, &tmp);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+- type_id = tmp;
+- if (!payload.empty()) {
++ if (state == State::kNoTag) {
++ type_id = tmp;
++ state = State::kHasType;
++ } else if (state == State::kHasPayload) {
++ type_id = tmp;
+ const FieldDescriptor* field;
+ if (ctx->data().pool == nullptr) {
+ field = reflection->FindKnownExtensionByNumber(type_id);
+@@ -697,16 +702,17 @@ struct WireFormat::MessageSetParser {
+ GOOGLE_PROTOBUF_PARSER_ASSERT(value->_InternalParse(p, &tmp_ctx) &&
+ tmp_ctx.EndedAtLimit());
+ }
+- type_id = 0;
++ state = State::kDone;
+ }
+ continue;
+ } else if (tag == WireFormatLite::kMessageSetMessageTag) {
+- if (type_id == 0) {
++ if (state == State::kNoTag) {
+ int32 size = ReadSize(&ptr);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+ ptr = ctx->ReadString(ptr, size, &payload);
+ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+- } else {
++ state = State::kHasPayload;
++ } else if (state == State::kHasType) {
+ // We're now parsing the payload
+ const FieldDescriptor* field = nullptr;
+ if (descriptor->IsExtensionNumber(type_id)) {
+@@ -720,7 +725,12 @@ struct WireFormat::MessageSetParser {
+ ptr = WireFormat::_InternalParseAndMergeField(
+ msg, ptr, ctx, static_cast<uint64>(type_id) * 8 + 2, reflection,
+ field);
+- type_id = 0;
++ state = State::kDone;
++ } else {
++ int32_t size = ReadSize(&ptr);
++ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
++ ptr = ctx->Skip(ptr, size);
++ GOOGLE_PROTOBUF_PARSER_ASSERT(ptr);
+ }
+ } else {
+ // An unknown field in MessageSetItem.
+diff --git a/src/google/protobuf/wire_format_lite.h b/src/google/protobuf/wire_format_lite.h
+index 5f5dd80f8..1af4d0625 100644
+--- a/src/google/protobuf/wire_format_lite.h
++++ b/src/google/protobuf/wire_format_lite.h
+@@ -1834,6 +1834,9 @@ bool ParseMessageSetItemImpl(io::CodedInputStream* input, MS ms) {
+ // we can parse it later.
+ std::string message_data;
+
++ enum class State { kNoTag, kHasType, kHasPayload, kDone };
++ State state = State::kNoTag;
++
+ while (true) {
+ const uint32 tag = input->ReadTagNoLastTag();
+ if (tag == 0) return false;
+@@ -1842,26 +1845,34 @@ bool ParseMessageSetItemImpl(io::CodedInputStream* input, MS ms) {
+ case WireFormatLite::kMessageSetTypeIdTag: {
+ uint32 type_id;
+ if (!input->ReadVarint32(&type_id)) return false;
+- last_type_id = type_id;
+-
+- if (!message_data.empty()) {
++ if (state == State::kNoTag) {
++ last_type_id = type_id;
++ state = State::kHasType;
++ } else if (state == State::kHasPayload) {
+ // We saw some message data before the type_id. Have to parse it
+ // now.
+ io::CodedInputStream sub_input(
+ reinterpret_cast<const uint8*>(message_data.data()),
+ static_cast<int>(message_data.size()));
+ sub_input.SetRecursionLimit(input->RecursionBudget());
+- if (!ms.ParseField(last_type_id, &sub_input)) {
++ if (!ms.ParseField(type_id, &sub_input)) {
+ return false;
+ }
+ message_data.clear();
++ state = State::kDone;
+ }
+
+ break;
+ }
+
+ case WireFormatLite::kMessageSetMessageTag: {
+- if (last_type_id == 0) {
++ if (state == State::kHasType) {
++ // Already saw type_id, so we can parse this directly.
++ if (!ms.ParseField(last_type_id, input)) {
++ return false;
++ }
++ state = State::kDone;
++ } else if (state == State::kNoTag) {
+ // We haven't seen a type_id yet. Append this data to message_data.
+ uint32 length;
+ if (!input->ReadVarint32(&length)) return false;
+@@ -1872,11 +1883,9 @@ bool ParseMessageSetItemImpl(io::CodedInputStream* input, MS ms) {
+ auto ptr = reinterpret_cast<uint8*>(&message_data[0]);
+ ptr = io::CodedOutputStream::WriteVarint32ToArray(length, ptr);
+ if (!input->ReadRaw(ptr, length)) return false;
++ state = State::kHasPayload;
+ } else {
+- // Already saw type_id, so we can parse this directly.
+- if (!ms.ParseField(last_type_id, input)) {
+- return false;
+- }
++ if (!ms.SkipField(tag, input)) return false;
+ }
+
+ break;
+diff --git a/src/google/protobuf/wire_format_unittest.inc b/src/google/protobuf/wire_format_unittest.inc
+index c2d2a00af..5653be796 100644
+--- a/src/google/protobuf/wire_format_unittest.cc
++++ b/src/google/protobuf/wire_format_unittest.cc
+@@ -46,6 +46,7 @@
+ #include <google/protobuf/io/zero_copy_stream_impl.h>
+ #include <google/protobuf/io/zero_copy_stream_impl_lite.h>
+ #include <google/protobuf/descriptor.h>
++#include <google/protobuf/dynamic_message.h>
+ #include <google/protobuf/wire_format_lite.h>
+ #include <google/protobuf/testing/googletest.h>
+ #include <gtest/gtest.h>
+@@ -581,28 +581,54 @@ TEST(WireFormatTest, ParseMessageSet) {
+ EXPECT_EQ(message_set.DebugString(), dynamic_message_set.DebugString());
+ }
+
+-TEST(WireFormatTest, ParseMessageSetWithReverseTagOrder) {
++namespace {
++std::string BuildMessageSetItemStart() {
+ std::string data;
+ {
+- unittest::TestMessageSetExtension1 message;
+- message.set_i(123);
+- // Build a MessageSet manually with its message content put before its
+- // type_id.
+ io::StringOutputStream output_stream(&data);
+ io::CodedOutputStream coded_output(&output_stream);
+ coded_output.WriteTag(WireFormatLite::kMessageSetItemStartTag);
++ }
++ return data;
++}
++std::string BuildMessageSetItemEnd() {
++ std::string data;
++ {
++ io::StringOutputStream output_stream(&data);
++ io::CodedOutputStream coded_output(&output_stream);
++ coded_output.WriteTag(WireFormatLite::kMessageSetItemEndTag);
++ }
++ return data;
++}
++std::string BuildMessageSetTestExtension1(int value = 123) {
++ std::string data;
++ {
++ unittest::TestMessageSetExtension1 message;
++ message.set_i(value);
++ io::StringOutputStream output_stream(&data);
++ io::CodedOutputStream coded_output(&output_stream);
+ // Write the message content first.
+ WireFormatLite::WriteTag(WireFormatLite::kMessageSetMessageNumber,
+ WireFormatLite::WIRETYPE_LENGTH_DELIMITED,
+ &coded_output);
+ coded_output.WriteVarint32(message.ByteSize());
+ message.SerializeWithCachedSizes(&coded_output);
+- // Write the type id.
+- uint32 type_id = message.GetDescriptor()->extension(0)->number();
++ }
++ return data;
++}
++std::string BuildMessageSetItemTypeId(int extension_number) {
++ std::string data;
++ {
++ io::StringOutputStream output_stream(&data);
++ io::CodedOutputStream coded_output(&output_stream);
+ WireFormatLite::WriteUInt32(WireFormatLite::kMessageSetTypeIdNumber,
+- type_id, &coded_output);
+- coded_output.WriteTag(WireFormatLite::kMessageSetItemEndTag);
++ extension_number, &coded_output);
+ }
++ return data;
++}
++void ValidateTestMessageSet(const std::string& test_case,
++ const std::string& data) {
++ SCOPED_TRACE(test_case);
+ {
+ proto2_wireformat_unittest::TestMessageSet message_set;
+ ASSERT_TRUE(message_set.ParseFromString(data));
+@@ -612,6 +638,11 @@ TEST(WireFormatTest, ParseMessageSetWithReverseTagOrder) {
+ .GetExtension(
+ unittest::TestMessageSetExtension1::message_set_extension)
+ .i());
++
++ // Make sure it does not contain anything else.
++ message_set.ClearExtension(
++ unittest::TestMessageSetExtension1::message_set_extension);
++ EXPECT_EQ(message_set.SerializeAsString(), "");
+ }
+ {
+ // Test parse the message via Reflection.
+@@ -627,6 +658,61 @@ TEST(WireFormatTest, ParseMessageSetWithReverseTagOrder) {
+ unittest::TestMessageSetExtension1::message_set_extension)
+ .i());
+ }
++ {
++ // Test parse the message via DynamicMessage.
++ DynamicMessageFactory factory;
++ std::unique_ptr<Message> msg(
++ factory
++ .GetPrototype(
++ proto2_wireformat_unittest::TestMessageSet::descriptor())
++ ->New());
++ msg->ParseFromString(data);
++ auto* reflection = msg->GetReflection();
++ std::vector<const FieldDescriptor*> fields;
++ reflection->ListFields(*msg, &fields);
++ ASSERT_EQ(fields.size(), 1);
++ const auto& sub = reflection->GetMessage(*msg, fields[0]);
++ reflection = sub.GetReflection();
++ EXPECT_EQ(123, reflection->GetInt32(
++ sub, sub.GetDescriptor()->FindFieldByName("i")));
++ }
++}
++} // namespace
++
++TEST(WireFormatTest, ParseMessageSetWithAnyTagOrder) {
++ std::string start = BuildMessageSetItemStart();
++ std::string end = BuildMessageSetItemEnd();
++ std::string id = BuildMessageSetItemTypeId(
++ unittest::TestMessageSetExtension1::descriptor()->extension(0)->number());
++ std::string message = BuildMessageSetTestExtension1();
++
++ ValidateTestMessageSet("id + message", start + id + message + end);
++ ValidateTestMessageSet("message + id", start + message + id + end);
++}
++
++TEST(WireFormatTest, ParseMessageSetWithDuplicateTags) {
++ std::string start = BuildMessageSetItemStart();
++ std::string end = BuildMessageSetItemEnd();
++ std::string id = BuildMessageSetItemTypeId(
++ unittest::TestMessageSetExtension1::descriptor()->extension(0)->number());
++ std::string other_id = BuildMessageSetItemTypeId(123456);
++ std::string message = BuildMessageSetTestExtension1();
++ std::string other_message = BuildMessageSetTestExtension1(321);
++
++ // Double id
++ ValidateTestMessageSet("id + other_id + message",
++ start + id + other_id + message + end);
++ ValidateTestMessageSet("id + message + other_id",
++ start + id + message + other_id + end);
++ ValidateTestMessageSet("message + id + other_id",
++ start + message + id + other_id + end);
++ // Double message
++ ValidateTestMessageSet("id + message + other_message",
++ start + id + message + other_message + end);
++ ValidateTestMessageSet("message + id + other_message",
++ start + message + id + other_message + end);
++ ValidateTestMessageSet("message + other_message + id",
++ start + message + other_message + id + end);
+ }
+
+ void SerializeReverseOrder(
+--
+2.40.0
+
diff --minimal -Nru protobuf-3.12.4/debian/patches/series protobuf-3.12.4/debian/patches/series
--- protobuf-3.12.4/debian/patches/series 2020-05-09 20:45:17.000000000 +0200
+++ protobuf-3.12.4/debian/patches/series 2023-04-04 11:41:41.000000000 +0200
@@ -9,3 +9,6 @@
32bit.patch
python3ify_examples.patch
no_errorprone.patch
+CVE-2021-22570.patch
+CVE-2021-22569.patch
+CVE-2022-1941.patch
Reply to: