Fix a rare crash from feature lifetime validation.

This is only reproducible for protos that trigger the lazy build of another proto in the pool that needs feature lifetimes validated.  If an error is encountered, we'll end up throwing the lazy build's descriptors into the deferred validation map, but then rolling it back with the original proto.  This results in a use-after-free crash.

PiperOrigin-RevId: 636324975
pull/16929/head
Mike Kruskal 6 months ago committed by Copybara-Service
parent 911fa984bc
commit 046a07eead
  1. 12
      src/google/protobuf/descriptor.cc
  2. 64
      src/google/protobuf/descriptor_unittest.cc

@ -1344,6 +1344,10 @@ class DescriptorPool::DeferredValidation {
lifetimes_info_map_[file].emplace_back(std::move(info)); lifetimes_info_map_[file].emplace_back(std::move(info));
} }
void RollbackFile(const FileDescriptor* file) {
lifetimes_info_map_.erase(file);
}
// Create a new file proto with an extended lifetime for deferred error // Create a new file proto with an extended lifetime for deferred error
// reporting. If any temporary file protos don't outlive this object, the // reporting. If any temporary file protos don't outlive this object, the
// reported errors won't be able to safely reference a location in the // reported errors won't be able to safely reference a location in the
@ -1446,7 +1450,7 @@ class DescriptorPool::Tables {
// Roll back the Tables to the state of the checkpoint at the top of the // Roll back the Tables to the state of the checkpoint at the top of the
// stack, removing everything that was added after that point. // stack, removing everything that was added after that point.
void RollbackToLastCheckpoint(); void RollbackToLastCheckpoint(DeferredValidation& deferred_validation);
// The stack of files which are currently being built. Used to detect // The stack of files which are currently being built. Used to detect
// cyclic dependencies when loading files from a DescriptorDatabase. Not // cyclic dependencies when loading files from a DescriptorDatabase. Not
@ -1638,7 +1642,8 @@ void DescriptorPool::Tables::ClearLastCheckpoint() {
} }
} }
void DescriptorPool::Tables::RollbackToLastCheckpoint() { void DescriptorPool::Tables::RollbackToLastCheckpoint(
DeferredValidation& deferred_validation) {
ABSL_DCHECK(!checkpoints_.empty()); ABSL_DCHECK(!checkpoints_.empty());
const CheckPoint& checkpoint = checkpoints_.back(); const CheckPoint& checkpoint = checkpoints_.back();
@ -1648,6 +1653,7 @@ void DescriptorPool::Tables::RollbackToLastCheckpoint() {
} }
for (size_t i = checkpoint.pending_files_before_checkpoint; for (size_t i = checkpoint.pending_files_before_checkpoint;
i < files_after_checkpoint_.size(); i++) { i < files_after_checkpoint_.size(); i++) {
deferred_validation.RollbackFile(files_after_checkpoint_[i]);
files_by_name_.erase(files_after_checkpoint_[i]); files_by_name_.erase(files_after_checkpoint_[i]);
} }
for (size_t i = checkpoint.pending_extensions_before_checkpoint; for (size_t i = checkpoint.pending_extensions_before_checkpoint;
@ -5822,7 +5828,7 @@ const FileDescriptor* DescriptorBuilder::BuildFile(
result->finished_building_ = true; result->finished_building_ = true;
alloc->ExpectConsumed(); alloc->ExpectConsumed();
} else { } else {
tables_->RollbackToLastCheckpoint(); tables_->RollbackToLastCheckpoint(deferred_validation_);
} }
return result; return result;

@ -12265,6 +12265,70 @@ TEST_F(DatabaseBackedPoolTest, FeatureLifetimeError) {
"pb.TestFeatures.future_feature wasn't introduced until edition 2024\n"); "pb.TestFeatures.future_feature wasn't introduced until edition 2024\n");
} }
TEST_F(DatabaseBackedPoolTest, FeatureLifetimeErrorUnknownDependencies) {
FileDescriptorProto proto;
FileDescriptorProto::descriptor()->file()->CopyTo(&proto);
std::string text_proto;
google::protobuf::TextFormat::PrintToString(proto, &text_proto);
AddToDatabase(&database_, text_proto);
pb::TestFeatures::descriptor()->file()->CopyTo(&proto);
google::protobuf::TextFormat::PrintToString(proto, &text_proto);
AddToDatabase(&database_, text_proto);
AddToDatabase(&database_, R"pb(
name: "option.proto"
syntax: "editions"
edition: EDITION_2023
dependency: "google/protobuf/descriptor.proto"
dependency: "google/protobuf/unittest_features.proto"
extension {
name: "foo_extension"
number: 1000
type: TYPE_STRING
extendee: ".google.protobuf.MessageOptions"
options {
features {
[pb.test] { legacy_feature: VALUE9 }
}
}
}
)pb");
// Note, we very carefully don't put a dependency here, otherwise option.proto
// will be built eagerly beforehand. This triggers a rare condition where
// DeferredValidation is filled with descriptors that are then rolled back.
AddToDatabase(&database_, R"pb(
name: "use_option.proto"
syntax: "editions"
edition: EDITION_2023
message_type {
name: "FooMessage"
options {
uninterpreted_option {
name { name_part: "foo_extension" is_extension: true }
string_value: "test"
}
}
field { name: "bar" number: 1 type: TYPE_INT64 }
}
)pb");
MockErrorCollector error_collector;
DescriptorPool pool(&database_, &error_collector);
ASSERT_EQ(pool.FindMessageTypeByName("FooMessage"), nullptr);
EXPECT_EQ(error_collector.text_,
"use_option.proto: FooMessage: OPTION_NAME: Option "
"\"(foo_extension)\" unknown. Ensure that your proto definition "
"file imports the proto which defines the option.\n");
// Verify that the extension does trigger a lifetime error.
error_collector.text_.clear();
ASSERT_EQ(pool.FindExtensionByName("foo_extension"), nullptr);
EXPECT_EQ(
error_collector.text_,
"option.proto: foo_extension: NAME: Feature "
"pb.TestFeatures.legacy_feature has been removed in edition 2023\n");
}
TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) { TEST_F(DatabaseBackedPoolTest, DoesntRetryDbUnnecessarily) {
// Searching for a child of an existing descriptor should never fall back // Searching for a child of an existing descriptor should never fall back
// to the DescriptorDatabase even if it isn't found, because we know all // to the DescriptorDatabase even if it isn't found, because we know all

Loading…
Cancel
Save