Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Crash if I click reopen database repeatedly from browser plugin #9938

Closed
Hugo-Leung opened this issue Oct 16, 2023 · 2 comments · Fixed by #9939
Closed

Crash if I click reopen database repeatedly from browser plugin #9938

Hugo-Leung opened this issue Oct 16, 2023 · 2 comments · Fixed by #9939

Comments

@Hugo-Leung
Copy link

Overview

KeePassXC would crash if I click the reopen database button in the browser plugin while KeePassXC is already decrypting the database.

Steps to Reproduce

  1. Set a large number of transform rounds
  2. Click reopen database on browser plug in
  3. enter master password
  4. Click reopen database again while KeePassXC is decrypting

Expected Behavior

The second reopen database command should be ignored

Actual Behavior

Crash with error
Screenshot 2023-10-16 092153

Context

I have also seen similar errors when exiting the application, similar to #7543. But the crash on exit seems to be more random, I have yet to find a reliable way the reproduce that.

KeePassXC - 2.7.6
Revision: dd21def

Operating System: Windows 11 Version 2009

@varjolintu
Copy link
Member

varjolintu commented Oct 16, 2023

For me it crashes here: https://github.com/keepassxreboot/keepassxc/blob/develop/src/core/Database.cpp#L821. Can anyone else reproduce this?

Maybe we are not prepared for a situation where multiple unlocks can happen simultaneously. Database class has a QMutex for saving operations. Maybe something similar could be done for database opening, but on global level. We could add some checks to BrowserService but it only fixes the problem on that end.

@droidmonkey
Copy link
Member

This is likely happening because the temporary Database instance in the unlock dialog is blown away once the database is actually unlocked. There are supposed to be safe guards to prevent that, but the browser service might be bypassing them using internal calls.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants