Skip to content

Commit ec3b59b

Browse files
julietlevesqueChromium LUCI CQ
authored and
Chromium LUCI CQ
committed
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}
1 parent 01a77f1 commit ec3b59b

File tree

2 files changed

+15
-5
lines changed

2 files changed

+15
-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

+5-1
Original file line numberDiff line numberDiff line change
@@ -43,12 +43,16 @@
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[6] = {
53+
static_cast<char>(0xFE), static_cast<char>(0xFE), static_cast<char>(0xFF),
54+
static_cast<char>(0xFF), static_cast<char>(0xFF), static_cast<char>(0xFF)};
55+
5256
const std::vector<uint8_t> kModelIdBytes = {
5357
/*message_group=*/0x03,
5458
/*message_code=*/0x01,

0 commit comments

Comments
 (0)