brave / brave-browser

Brave browser for Android, iOS, Linux, macOS, Windows.
https://brave.com
Mozilla Public License 2.0
17.04k stars 2.23k forks source link

Brave crashes importing passwords from MS Edge #32939

Open Brave-Matt opened 10 months ago

Brave-Matt commented 10 months ago

Description

Crash report IDs: Some users have reported an issue with Brave on Windows 11 where the browser will crash after attempting to import passwords from MS Edge browser.

471d0400-30cd-ae0a-0000-000000000000
461d0400-30cd-ae0a-0000-000000000000
441d0400-30cd-ae0a-0000-000000000000

cc @iefremov

Steps to Reproduce

  1. Visit brave://settings/importData
  2. Select Microsoft Edge Person 1
  3. Uncheck all except Saved passwords
  4. Click Import

Actual result:

Browser crashes

Expected result:

Passwords should be imported

Miscellaneous Information:

User thread: https://community.brave.com/t/bug-import-of-saved-passwords-from-microsoft-edge-causes-brave-to-crash/502254/4?u=mattches

bsclifton commented 4 months ago

Backtrace link: https://brave.sp.backtrace.io/p/brave/triage?time=year&filters=JTVCJTVCJTIyX3J4aWQlMjIlMkMlMjJlcXVhbCUyMiUyQyUyMjQ3MWQwNDAwLTMwY2QtYWUwYS0wMDAwLTAwMDAwMDAwMDAwMCUyMiU1RCU1RA%3D%3D&aggregations=((guid%2Cunique)%2C(classifiers%2Chead))&fingerprint=3e11722208a95f08091fd05f39004a0d983e9db6ff2a0ddce84ffaf666fdd4b2&similarity=false

[ 00 ] base::subtle::RefCountedThreadSafeBase::AddRefWithCheckImpl
[ 01 ] password_manager::PasswordStore::AddLogins(std::__Cr::vector<password_manager::PasswordForm,std::__Cr::allocator<password_manager::PasswordForm> > const &,base::OnceCallback<void ()>)
[ 02 ] password_manager::PasswordStore::AddLogin(password_manager::PasswordForm const &,base::OnceCallback<void ()>)
[ 03 ] ProfileWriter::AddPasswordForm
[ 04 ] InProcessImporterBridge::SetPasswordForm
[ 05 ] chrome::mojom::ProfileImportObserverStubDispatch::Accept
[ 06 ] mojo::InterfaceEndpointClient::HandleValidatedMessage
[ 07 ] mojo::InterfaceEndpointClient::HandleIncomingMessageThunk::Accept
[ 08 ] mojo::internal::MultiplexRouter::ProcessIncomingMessage
[ 09 ] mojo::internal::MultiplexRouter::Accept
[ 10 ] mojo::MessageDispatcher::Accept
[ 11 ] mojo::Connector::ReadAllAvailableMessages
[ 12 ] mojo::Connector::CallDispatchNextMessageFromPipe
[ 13 ] content::BrowserMainLoop::RunMainMessageLoop
[ 14 ] content::BrowserMainRunnerImpl::Run
[ 15 ] content::BrowserMain
[ 16 ] RunBrowserProcessMain
[ 17 ] content::ContentMainRunnerImpl::RunBrowser
[ 18 ] content::ContentMainRunnerImpl::Run
[ 19 ] RunContentProcess
[ 20 ] content::ContentMain
[ 21 ] ChromeMain
[ 22 ] MainDllLoader::Launch
[ 23 ] wWinMain
bsclifton commented 4 months ago

Not able to reproduce on Windows 11 x64 using Edge 121.0.2277.128 and importing passwords into Brave 1.63.161. It pulled them just fine for me. It does appear I'm using sync though (may be configured by default).

I turned OFF sync and tried saving another password. It imports just fine too.

I suspect there may be an issue trying to read the credentials if they are encrypted with a key or secret that has expired (ex: admin on network forced PW change for person). We might not handle an undecodable secret properly on our side (in Import).

Relevant code: https://github.com/brave/brave-core/blob/7b27df7afeabe7aff5e3cc93afabb77379a1975e/utility/importer/chrome_importer.cc#L454-L490