Skip to content

🔒 Fix search information leak#603

Merged
phoerious merged 1 commit intokeepassxreboot:developfrom
hifi:fix-search-leak
May 28, 2017
Merged

🔒 Fix search information leak#603
phoerious merged 1 commit intokeepassxreboot:developfrom
hifi:fix-search-leak

Conversation

@hifi
Copy link
Copy Markdown
Contributor

@hifi hifi commented May 28, 2017

Description

When databases are locked, end search mode if it was activated and clear the visible input field for search term.

The DatabaseWidget::endSearch() call on DatabaseWidget::lock() clears the search term from memory regardless if search mode was active or not.

Adding a default argument for SearchWidget::databaseChanged() allows using the existing method for lock signaling.

Motivation and context

A user on IRC pointed out that the search field is not cleared when locking databases and it could be perceived as unintentional information leak.

@TheZ3ro said the maintainers are considering this but no issue was ever opened.

How has this been tested?

Locking/unlocking databases multiple databases, switching databases.

Types of changes

  • ✅ Bug fix (non-breaking change which fixes an issue)

Checklist:

  • ✅ I have read the CONTRIBUTING document. [REQUIRED]
  • ✅ My code follows the code style of this project. [REQUIRED]

@TheZ3ro
Copy link
Copy Markdown
Contributor

TheZ3ro commented May 28, 2017

The thing is that in some use-case, the user want to maintain the search term when the db is locked to reload the search when it's unlocked. Maybe we should handle also this situation.

@phoerious
Copy link
Copy Markdown
Member

I think it's fine to just delete the field contents. Whatever that other use case would be, security comes first.

@TheZ3ro
Copy link
Copy Markdown
Contributor

TheZ3ro commented May 28, 2017

@hifi can you rebase this on top of latest develop?

@hifi hifi force-pushed the fix-search-leak branch from f10d878 to 6f4b5fc Compare May 28, 2017 15:08
@hifi
Copy link
Copy Markdown
Contributor Author

hifi commented May 28, 2017

Done.

@phoerious phoerious merged commit 79ba3a2 into keepassxreboot:develop May 28, 2017
@phoerious phoerious added this to the v2.2.0 milestone May 28, 2017
droidmonkey added a commit that referenced this pull request Jun 25, 2017
- Added YubiKey 2FA integration for unlocking databases [#127]
- Added TOTP support [#519]
- Added CSV import tool [#146, #490]
- Added KeePassXC CLI tool [#254]
- Added diceware password generator [#373]
- Added support for entry references [#370, #378]
- Added support for Twofish encryption [#167]
- Enabled DEP and ASLR for in-memory protection [#371]
- Enabled single instance mode [#510]
- Enabled portable mode [#645]
- Enabled database lock on screensaver and session lock [#545]
- Redesigned welcome screen with common features and recent databases [#292]
- Multiple updates to search behavior [#168, #213, #374, #471, #603, #654]
- Added auto-type fields {CLEARFIELD}, {SPACE}, {{}, {}} [#267, #427, #480]
- Fixed auto-type errors on Linux [#550]
- Prompt user prior to executing a cmd:// URL [#235]
- Entry attributes can be protected (hidden) [#220]
- Added extended ascii to password generator [#538]
- Added new database icon to toolbar [#289]
- Added context menu entry to empty recycle bin in databases [#520]
- Added "apply" button to entry and group edit windows [#624]
- Added macOS tray icon and enabled minimize on close [#583]
- Fixed issues with unclean shutdowns [#170, #580]
- Changed keyboard shortcut to create new database to CTRL+SHIFT+N [#515]
- Compare window title to entry URLs [#556]
- Implemented inline error messages [#162]
- Ignore group expansion and other minor changes when making database "dirty" [#464]
- Updated license and copyright information on souce files [#632]
- Added contributors list to about dialog [#629]
@phoerious phoerious added pr: bugfix Pull request fixes a bug and removed bug labels Nov 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr: bugfix Pull request fixes a bug security

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants