Package: release.debian.org Severity: normal Tags: bullseye User: release.debian....@packages.debian.org Usertags: pu X-Debbugs-Cc: proto...@packages.debian.org, g...@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 <hel...@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 <acozze...@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 <deannagar...@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 <mkrus...@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