From 2e00758ff8c984c8ad1dc4f5bd6e1ff7eee5856e Mon Sep 17 00:00:00 2001 From: wenlong12 Date: Mon, 21 Mar 2022 17:33:03 +0800 Subject: [PATCH] fix -CVE-2021-22570 issue Signed-off-by: wenlong12 Signed-off-by: wenlong12 --- src/google/protobuf/descriptor.cc | 25 ++++++++- src/google/protobuf/descriptor_unittest.cc | 65 ++++++++++++++++++++++ 2 files changed, 88 insertions(+), 2 deletions(-) diff --git a/src/google/protobuf/descriptor.cc b/src/google/protobuf/descriptor.cc index 9a448ff..40510b4 100644 --- a/src/google/protobuf/descriptor.cc +++ b/src/google/protobuf/descriptor.cc @@ -1090,7 +1090,7 @@ inline void DescriptorPool::Tables::FindAllExtensions( bool DescriptorPool::Tables::AddSymbol(const std::string& full_name, Symbol symbol) { - if (InsertIfNotPresent(&symbols_by_name_, full_name.c_str(), symbol)) { + if (InsertIfNotPresent(&symbols_by_name_, full_name, symbol)) { symbols_after_checkpoint_.push_back(full_name.c_str()); return true; } else { @@ -1106,7 +1106,7 @@ bool FileDescriptorTables::AddAliasUnderParent(const void* parent, } bool DescriptorPool::Tables::AddFile(const FileDescriptor* file) { - if (InsertIfNotPresent(&files_by_name_, file->name().c_str(), file)) { + if (InsertIfNotPresent(&files_by_name_, file->name(), file)) { files_after_checkpoint_.push_back(file->name().c_str()); return true; } else { @@ -2628,6 +2628,8 @@ void Descriptor::DebugString(int depth, std::string* contents, const Descriptor::ReservedRange* range = reserved_range(i); if (range->end == range->start + 1) { strings::SubstituteAndAppend(contents, "$0, ", range->start); + } else if (range->end > FieldDescriptor::kMaxNumber) { + strings::SubstituteAndAppend(contents, "$0 to max, ", range->start); } else { strings::SubstituteAndAppend(contents, "$0 to $1, ", range->start, range->end - 1); @@ -2831,6 +2833,8 @@ void EnumDescriptor::DebugString( const EnumDescriptor::ReservedRange* range = reserved_range(i); if (range->end == range->start) { strings::SubstituteAndAppend(contents, "$0, ", range->start); + } else if (range->end == INT_MAX) { + strings::SubstituteAndAppend(contents, "$0 to max, ", range->start); } else { strings::SubstituteAndAppend(contents, "$0 to $1, ", range->start, range->end); @@ -4022,6 +4026,12 @@ bool DescriptorBuilder::AddSymbol(const std::string& full_name, // 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 @@ -4061,6 +4071,11 @@ bool DescriptorBuilder::AddSymbol(const std::string& full_name, 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('.'); @@ -4374,6 +4389,12 @@ FileDescriptor* DescriptorBuilder::BuildFileImpl( } 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 --git a/src/google/protobuf/descriptor_unittest.cc b/src/google/protobuf/descriptor_unittest.cc index 6085a12..56c180a 100644 --- a/src/google/protobuf/descriptor_unittest.cc +++ b/src/google/protobuf/descriptor_unittest.cc @@ -3786,6 +3786,45 @@ TEST_F(ValidationErrorTest, InvalidPackageName) { "foo.proto: foo.$: NAME: \"$\" is not a valid identifier.\n"); } +// 'str' is a static C-style string that may contain '\0' +#define STATIC_STR(str) std::string((str), sizeof(str) - 1) + +TEST_F(ValidationErrorTest, NullCharSymbolName) { + BuildFileWithErrors( + "name: \"bar.proto\" " + "package: \"foo\"" + "message_type { " + " name: '\\000\\001\\013.Bar' " + " field { name: \"foo\" number: 9 label:LABEL_OPTIONAL type:TYPE_INT32 " + "} " + "}", + STATIC_STR("bar.proto: foo.\0\x1\v.Bar: NAME: \"\0\x1\v.Bar\" is not a " + "valid identifier.\nbar.proto: foo.\0\x1\v.Bar: NAME: " + "\"\0\x1\v.Bar\" is not a valid identifier.\nbar.proto: " + "foo.\0\x1\v.Bar: NAME: \"\0\x1\v.Bar\" is not a valid " + "identifier.\nbar.proto: foo.\0\x1\v.Bar: NAME: " + "\"\0\x1\v.Bar\" is not a valid identifier.\nbar.proto: " + "foo.\0\x1\v.Bar.foo: NAME: \"foo.\0\x1\v.Bar.foo\" contains " + "null character.\nbar.proto: foo.\0\x1\v.Bar: NAME: " + "\"foo.\0\x1\v.Bar\" contains null character.\n")); +} + +TEST_F(ValidationErrorTest, NullCharFileName) { + BuildFileWithErrors( + "name: \"bar\\000\\001\\013.proto\" " + "package: \"outer.foo\"", + STATIC_STR("bar\0\x1\v.proto: bar\0\x1\v.proto: NAME: " + "\"bar\0\x1\v.proto\" contains null character.\n")); +} + +TEST_F(ValidationErrorTest, NullCharPackageName) { + BuildFileWithErrors( + "name: \"bar.proto\" " + "package: \"\\000\\001\\013.\"", + STATIC_STR("bar.proto: \0\x1\v.: NAME: \"\0\x1\v.\" contains null " + "character.\n")); +} + TEST_F(ValidationErrorTest, MissingFileName) { BuildFileWithErrors("", @@ -4001,6 +4040,32 @@ TEST_F(ValidationErrorTest, ReservedFieldsDebugString) { file->DebugString()); } +TEST_F(ValidationErrorTest, DebugStringReservedRangeMax) { + const FileDescriptor* file = BuildFile(strings::Substitute( + "name: \"foo.proto\" " + "enum_type { " + " name: \"Bar\"" + " value { name:\"BAR\" number:1 }" + " reserved_range { start: 5 end: $0 }" + "}" + "message_type {" + " name: \"Foo\"" + " reserved_range { start: 5 end: $1 }" + "}", + std::numeric_limits::max(), FieldDescriptor::kMaxNumber + 1)); + + ASSERT_EQ( + "syntax = \"proto2\";\n\n" + "enum Bar {\n" + " BAR = 1;\n" + " reserved 5 to max;\n" + "}\n\n" + "message Foo {\n" + " reserved 5 to max;\n" + "}\n\n", + file->DebugString()); +} + TEST_F(ValidationErrorTest, EnumReservedFieldError) { BuildFileWithErrors( "name: \"foo.proto\" " -- Gitee