Skip to content

Commit 2ea9fdd

Browse files
julietlevesqueChromium LUCI CQ
authored and
Chromium LUCI CQ
committed
[Merge M111] Reland "Reland "[Fast Pair] Prevent invalid iterators when removing expired devices""
This is a reland of commit ec3b59b Instead of using a char[] array, changed to use a string literal to avoid breaking due to buffer overflow when representing the invalid UTF8 string. Verified that this passes the CQ asan bot. Original change's description: > Reland "[Fast Pair] Prevent invalid iterators when removing expired devices" > > This is a reland of commit 6b6ced5 > This change fixes the global buffer overflow by explicitly saying > the test address bytes size is 6. > > Original change's description: > > [Fast Pair] Prevent invalid iterators when removing expired devices > > > > When iterating over the list of devices, we want to also remove from > > this list, however this can invalidate the iterator we are using to > > parse over them. This change uses the classic approach of loop over > > the map and collect the keys you want to remove in the vector, the > > loop over the vector and removes them. > > > > tested on DUT that retroactive pairing and removing expired devices > > works as expected. > > > > Test: Recreated the crash with unit tests and verified it works, > > Fixed: b/266753250 > > Change-Id: If1e36070facf5d599b248d29ed944bc581668a05 > > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4195321 > > Reviewed-by: Daniel Classon <dclasson@google.com> > > Commit-Queue: Juliet Lévesque <julietlevesque@google.com> > > Cr-Commit-Position: refs/heads/main@{#1097185} > > Change-Id: I71be79695548f7caf930219196c49201bd35f064 > Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4197855 > Commit-Queue: Juliet Lévesque <julietlevesque@google.com> > Reviewed-by: Daniel Classon <dclasson@google.com> > Cr-Commit-Position: refs/heads/main@{#1097447} (cherry picked from commit 4aadc25) Change-Id: If223c3c3abdb6b47e9f92fa2fac4ff8fc5588e25 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4198835 Reviewed-by: Daniel Classon <dclasson@google.com> Commit-Queue: Juliet Lévesque <julietlevesque@google.com> Cr-Original-Commit-Position: refs/heads/main@{#1097686} Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/4220260 Reviewed-by: Ryan Hansberry <hansberry@chromium.org> Cr-Commit-Position: refs/branch-heads/5563@{#128} Cr-Branched-From: 3ac59a6-refs/heads/main@{#1097615}
1 parent 4c1558f commit 2ea9fdd

File tree

2 files changed

+13
-5
lines changed

2 files changed

+13
-5
lines changed

ash/quick_pair/pairing/retroactive_pairing_detector_impl.cc

+10-4
Original file line numberDiff line numberDiff line change
@@ -491,6 +491,7 @@ void RetroactivePairingDetectorImpl::RemoveDeviceInformation(
491491

492492
void RetroactivePairingDetectorImpl::RemoveDeviceInformationHelper(
493493
const std::string& device_address) {
494+
QP_LOG(INFO) << __func__;
494495
potential_retroactive_addresses_.erase(device_address);
495496
device_pairing_information_.erase(device_address);
496497

@@ -511,15 +512,20 @@ void RetroactivePairingDetectorImpl::
511512
// `CheckPairingInformation` if it has exceeded the allotted time for
512513
// detecting the scenario (kDetectRetroactiveScenarioTimeout). We clean up
513514
// these devices here.
515+
std::vector<std::string> devices_to_remove;
514516
for (auto it = device_pairing_information_.begin();
515517
it != device_pairing_information_.end(); ++it) {
516518
if (base::Time::Now() >= it->second.expiry_timestamp) {
517-
QP_LOG(VERBOSE) << __func__ << ": Removing device at " << it->first
518-
<< "that has exceeded the time allotted for detecting "
519-
"retroactive scenario.";
520-
RemoveDeviceInformationHelper(it->first);
519+
devices_to_remove.push_back(it->first);
521520
}
522521
}
522+
523+
for (std::string device_address : devices_to_remove) {
524+
QP_LOG(VERBOSE) << __func__ << ": Removing device at " << device_address
525+
<< "that has exceeded the time allotted for detecting "
526+
"retroactive scenario.";
527+
RemoveDeviceInformationHelper(device_address);
528+
}
523529
}
524530

525531
void RetroactivePairingDetectorImpl::OnPairFailure(scoped_refptr<Device> device,

ash/quick_pair/pairing/retroactive_pairing_detector_unittest.cc

+3-1
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,14 @@
4343
namespace {
4444

4545
constexpr base::TimeDelta kRetroactiveDevicePairingTimeout = base::Seconds(60);
46-
constexpr char kTestDeviceAddress[] = "11:12:13:14:15:16";
4746
constexpr char kTestDeviceAddress2[] = "11:12:13:14:15:17";
4847
constexpr char kTestBleDeviceName[] = "Test Device Name";
4948
constexpr char kValidModelId[] = "718c17";
5049
const std::string kUserEmail = "test@test.test";
5150

51+
// Expect no crash with invalid UTF-8 strings as device address.
52+
constexpr char kTestDeviceAddress[] = "\0xff\0xff\0xff\0xff\0xff\0xff";
53+
5254
const std::vector<uint8_t> kModelIdBytes = {
5355
/*message_group=*/0x03,
5456
/*message_code=*/0x01,

0 commit comments

Comments
 (0)