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

Fix heap-use-after-free & error-msg output in keepassxc-cli #5368

Conversation

Colfenor
Copy link
Contributor

@Colfenor Colfenor commented Sep 1, 2020

Proposed fix for #5367

After investigating with AddressSanitizer it seems that
the ~Database destructor is called twice, since the QSharedPointer
variable cmd->currentDatabase & currentDatabase is initialized to
the the same address.

also fixed a issue with the cli unkown command output, since
in case of a wrong command only a "\n" character is appended to the
Utils::STDERR textstream but not flushed & printed.
Upon exitting the shell the stream is flushed and if a bunch of unkown
commands are entered all of them are subsequently printed.

With endl the newline char is appended and the error-message is flushed,
printed and the shell continues.

Testing strategy

  1. Open a .kdbx database file with the keepassxc-cli open cmd
  2. Enter any valid command as defined in the namespace commands
  3. Exit the shell by either "exit" or "quit"

Screenshots

Type of change

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

src/cli/keepassxc-cli.cpp Outdated Show resolved Hide resolved
@Colfenor Colfenor requested a review from droidmonkey September 3, 2020 16:09
@droidmonkey droidmonkey changed the base branch from develop to release/2.6.2 September 4, 2020 22:01
@droidmonkey droidmonkey force-pushed the hotfix/fix_heapUseAfterFree_ branch from c229cd5 to e6b98a7 Compare September 4, 2020 22:06
@droidmonkey droidmonkey merged commit 9cf9311 into keepassxreboot:release/2.6.2 Sep 5, 2020
phoerious added a commit that referenced this pull request Oct 21, 2020
Added

- Add option to keep window always on top to view menu [#5542]
- Move show/hide usernames and passwords to view menu [#5542]
- Add command line options and environment variables for changing the config locations [#5452]
- Include TOTP settings in CSV import/export and add support for ISO datetimes [#5346]

Changed

- Mask sensitive information in command execution confirmation prompt [#5542]
- SSH Agent: Avoid shortcut conflict on macOS by changing "Add key" to Ctrl+H on all platforms [#5484]

Fixed

- Prevent data loss with drag and drop between databases [#5536]
- Fix crash when toggling Capslock rapidly [#5545]
- Don't mark URL references as invalid URL [#5380]
- Reset entry preview after search [#5483]
- Set Qt::Dialog flag on database open dialog [#5356]
- Fix sorting of database report columns [#5426]
- Fix IfDevice matching logic [#5344]
- Fix layout issues and a stray scrollbar appearing on top of the entry edit screen [#5424]
- Fix tabbing into the notes field [#5424]
- Fix password generator ignoring settings on load [#5340]
- Restore natural entry sort order on application load [#5438]
- Fix paperclip and TOTP columns not saving state [#5327]
- Enforce fixed password font in entry preview [#5454]
- Add scrollbar when new database wizard exceeds screen size [#5560]
- Do not mark database as modified when viewing Auto-Type associations [#5542]
- CLI: Fix two heap-use-after-free crashes [#5368,#5470]
- Browser: Fix key exchange not working with multiple simultaneous users on Windows [#5485]
- Browser: Fix entry retrieval when "only best matching" is enabled [#5316]
- Browser: Ignore recycle bin on KeePassHTTP migration [#5481]
- KeeShare: Fix import crash [#5542]
- macOS: Fix toolbar theming and breadcrumb display issues [#5482]
- macOS: Fix file dialog randomly closing [#5479]
- macOS: Fix being unable to select OPVault files for import [#5341]
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants