Skip to content

Commit

Permalink
Fix dangling pointer in HttpsOnlyMode[Allow|Enforce]list
Browse files Browse the repository at this point in the history
StatefulSSLHostStateDelegate owns a HttpsOnlyModeAllowlist and a
HttpsOnlyModeEnforcelist object, passing them a pointer to the Clock
that it owns. However, when setting a Clock object in testing via
StatefulSSLHostStateDelegate::SetClockForTesting(), the pointers held
by the HttpsOnlyModeAllowlist and HttpsOnlyModeEnforcelist would very
briefly be dangling between the Delegate taking ownership of the new
Clock object (causing the old Clock object to be destroyed) and a new
raw pointer to the new Clock object being passed into the Allowlist
and Enforcelist objects. This fixes that brief dangling (which caused
the DanglingPtr detector to trigger) by resetting the Clock pointer in
the Allowlist/Enforcelist to nullptr before setting the new Clock
object.

Bug: 1442617
Change-Id: I76a5138dcc4c384f9bfc3045b79e2773fad13080
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4727236
Auto-Submit: Chris Thompson <[email protected]>
Reviewed-by: Mustafa Emre Acer <[email protected]>
Commit-Queue: Mustafa Emre Acer <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1176564}
  • Loading branch information
christhompson authored and Chromium LUCI CQ committed Jul 28, 2023
1 parent 0f7a6a6 commit bd0f22a
Show file tree
Hide file tree
Showing 3 changed files with 9 additions and 4 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -528,9 +528,14 @@ void StatefulSSLHostStateDelegate::ResetRecurrentErrorCountForTesting() {

void StatefulSSLHostStateDelegate::SetClockForTesting(
std::unique_ptr<base::Clock> clock) {
// Pointers to the existing Clock object must be reset before swapping the
// underlying Clock object, otherwise they are dangling (briefly).
https_only_mode_allowlist_.SetClockForTesting(nullptr); // IN-TEST
https_only_mode_enforcelist_.SetClockForTesting(nullptr); // IN-TEST

clock_ = std::move(clock);
https_only_mode_allowlist_.SetClockForTesting(clock_.get());
https_only_mode_enforcelist_.SetClockForTesting(clock_.get());
https_only_mode_allowlist_.SetClockForTesting(clock_.get()); // IN-TEST
https_only_mode_enforcelist_.SetClockForTesting(clock_.get()); // IN-TEST
}

void StatefulSSLHostStateDelegate::SetRecurrentInterstitialThresholdForTesting(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class HttpsOnlyModeAllowlist {

private:
raw_ptr<HostContentSettingsMap> host_content_settings_map_;
raw_ptr<base::Clock, DanglingUntriaged> clock_;
raw_ptr<base::Clock> clock_;
base::TimeDelta expiration_timeout_;

// Tracks sites that are allowed to load over HTTP when HTTPS-First Mode is
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ class HttpsOnlyModeEnforcelist {
void RecordMetrics(bool is_nondefault_storage);

raw_ptr<HostContentSettingsMap> host_content_settings_map_;
raw_ptr<base::Clock, DanglingUntriaged> clock_;
raw_ptr<base::Clock> clock_;

// Tracks sites that are not allowed to load over HTTP, for non-default
// storage partitions. Enforced hosts are exact hostname matches -- subdomains
Expand Down

0 comments on commit bd0f22a

Please sign in to comment.