Commit 2312af4d authored by Martin Kreichgauer's avatar Martin Kreichgauer Committed by Commit Bot

fido: VirtualFidoDiscoveryFactory fixes

CreateAuthenticator() relies on VirtualAuthenticator IDs being unique,
which they are. But if they weren't, it would first dereference and then
return a dangling VirtualAuthenticator pointer. Since a similar code
piece has recently shown that same bug, make this method more defensive
so it doesn't regress under future changes.

Also fix a possible nullptr dereference.

Change-Id: Ia769929f94360eabcdadb5e0f706f83d031231f4
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2209905
Commit-Queue: Martin Kreichgauer <martinkr@google.com>
Auto-Submit: Martin Kreichgauer <martinkr@google.com>
Reviewed-by: default avatarNina Satragno <nsatragno@chromium.org>
Cr-Commit-Position: refs/heads/master@{#770768}
parent 6cc2c589
...@@ -50,8 +50,15 @@ VirtualAuthenticator* VirtualFidoDiscoveryFactory::CreateAuthenticator( ...@@ -50,8 +50,15 @@ VirtualAuthenticator* VirtualFidoDiscoveryFactory::CreateAuthenticator(
auto authenticator = std::make_unique<VirtualAuthenticator>( auto authenticator = std::make_unique<VirtualAuthenticator>(
protocol, transport, attachment, has_resident_key, has_user_verification); protocol, transport, attachment, has_resident_key, has_user_verification);
auto* authenticator_ptr = authenticator.get(); auto* authenticator_ptr = authenticator.get();
authenticators_.emplace(authenticator_ptr->unique_id(), bool was_inserted;
std::move(authenticator)); std::tie(std::ignore, was_inserted) = authenticators_.insert(
{authenticator_ptr->unique_id(), std::move(authenticator)});
if (!was_inserted) {
// unique_id() is unique, so map insertion should succeed. But let's be
// paranoid so we don't accidentally return a dangling pointer.
NOTREACHED();
return nullptr;
}
for (auto* discovery : discoveries_) { for (auto* discovery : discoveries_) {
if (discovery->transport() != authenticator_ptr->transport()) if (discovery->transport() != authenticator_ptr->transport())
...@@ -119,6 +126,10 @@ void VirtualFidoDiscoveryFactory::CreateAuthenticator( ...@@ -119,6 +126,10 @@ void VirtualFidoDiscoveryFactory::CreateAuthenticator(
auto* authenticator = CreateAuthenticator( auto* authenticator = CreateAuthenticator(
options->protocol, options->transport, options->attachment, options->protocol, options->transport, options->attachment,
options->has_resident_key, options->has_user_verification); options->has_resident_key, options->has_user_verification);
if (!authenticator) {
std::move(callback).Run(mojo::NullRemote());
return;
}
authenticator->SetUserPresence(options->is_user_present); authenticator->SetUserPresence(options->is_user_present);
std::move(callback).Run(GetMojoToVirtualAuthenticator(authenticator)); std::move(callback).Run(GetMojoToVirtualAuthenticator(authenticator));
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment