Skip to content

Commit

Permalink
[Fuzzing] Fix use-after-free in resolver_fuzzer (grpc#33553)
Browse files Browse the repository at this point in the history
In FuzzingDNSResolver, capturing the engine as raw pointers in the
lambda functions instead of capturing the `this` pointer. By the time
the lambda is ran, the FuzzingDNSResolver might already be destroyed but
the engine should still be alive.


<!--

If you know who should review your pull request, please assign it to
that
person, otherwise the pull request would get assigned randomly.

If your pull request is for a specific language, please add the
appropriate
lang label.

-->
  • Loading branch information
yijiem authored Jun 26, 2023
1 parent 4b55f22 commit e3c22b9
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 20 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -154,46 +154,50 @@ class FuzzingResolverEventEngine
void LookupHostname(LookupHostnameCallback on_resolve,
absl::string_view /* name */,
absl::string_view /* default_port */) override {
CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_HOSTNAME);
engine_->CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_HOSTNAME);
if (!engine_->has_been_orphaned_) {
engine_->runner_.Run([this, cb = std::move(on_resolve)]() mutable {
CheckAndSetOrphan(ExecutionStep::AFTER_LOOKUP_HOSTNAME_CALLBACK);
cb(engine_->hostname_responses_);
});
engine_->runner_.Run(
[engine = engine_, cb = std::move(on_resolve)]() mutable {
engine->CheckAndSetOrphan(
ExecutionStep::AFTER_LOOKUP_HOSTNAME_CALLBACK);
cb(engine->hostname_responses_);
});
}
}
void LookupSRV(LookupSRVCallback on_resolve,
absl::string_view /* name */) override {
CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_SRV);
engine_->CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_SRV);
if (!engine_->has_been_orphaned_) {
engine_->runner_.Run([this, cb = std::move(on_resolve)]() mutable {
CheckAndSetOrphan(ExecutionStep::AFTER_LOOKUP_SRV_CALLBACK);
cb(engine_->srv_responses_);
engine_->runner_.Run([engine = engine_,
cb = std::move(on_resolve)]() mutable {
engine->CheckAndSetOrphan(ExecutionStep::AFTER_LOOKUP_SRV_CALLBACK);
cb(engine->srv_responses_);
});
}
}
void LookupTXT(LookupTXTCallback on_resolve,
absl::string_view /* name */) override {
CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_TXT);
engine_->CheckAndSetOrphan(ExecutionStep::DURING_LOOKUP_TXT);
if (!engine_->has_been_orphaned_) {
engine_->runner_.Run([this, cb = std::move(on_resolve)]() mutable {
CheckAndSetOrphan(ExecutionStep::AFTER_LOOKUP_TXT_CALLBACK);
cb(engine_->txt_responses_);
engine_->runner_.Run([engine = engine_,
cb = std::move(on_resolve)]() mutable {
engine->CheckAndSetOrphan(ExecutionStep::AFTER_LOOKUP_TXT_CALLBACK);
cb(engine->txt_responses_);
});
}
}

private:
void CheckAndSetOrphan(ExecutionStep current_execution_step) {
if (engine_->should_orphan_at_step_ == current_execution_step) {
*engine_->done_resolving_ = true;
engine_->has_been_orphaned_ = true;
}
}

FuzzingResolverEventEngine* engine_;
};

void CheckAndSetOrphan(ExecutionStep current_execution_step) {
if (should_orphan_at_step_ == current_execution_step) {
*done_resolving_ = true;
has_been_orphaned_ = true;
}
}

// members
FuzzingEventEngine runner_;
bool* done_resolving_;
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
channel_args {
args {
key: "grpc.dns_enable_srv_queries"
i: 1
}
}
should_orphan_at_step: DURING_LOOKUP_SRV
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
channel_args {
args {
key: "grpc.service_config_disable_resolution"
i: 0
}
}
should_orphan_at_step: DURING_LOOKUP_TXT

0 comments on commit e3c22b9

Please sign in to comment.