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

Cancelling favicon download causes a crash #1936

Closed
0x11de784a opened this issue May 10, 2018 · 11 comments
Closed

Cancelling favicon download causes a crash #1936

0x11de784a opened this issue May 10, 2018 · 11 comments
Assignees
Milestone

Comments

@0x11de784a
Copy link

Expected Behavior

While editing an entry clicking 'Download favicon' (always was and still) should download the favicon of the indicated URL.

Current Behavior

The download starts and the icon gets added to the custom icons field. But there are 1-3 download progress windows/pop ups in the foreground which are stuck at 100%. Their 'Cancel' button is not working. So pressing the window's close-window-x on one those leads to a segfault of the whole application.

Possible Solution

Meta: A --verbose or --debug command would help to track it down; I couldn't find such an option for keepassxc. Right now that issue could be a packaging problem within my distribution too, but there's not really a way for me to know that.

Steps to Reproduce (for bugs)

  1. Go to or add new entry.
  2. Add URL to entry. (I think full domain incl https is necessary)
  3. Change to 'Icon' tab and click 'Download favicon'.

Context

What I would be afraid of is: When adding a new entry incl. generated password and being still in the process of editing, this segfault could lead to corrupt data and maybe inaccessible accounts.

Debug Info

KeePassXC - Version 2.3.3
Revision: 0a155d8

Libraries:

  • Qt 5.10.1
  • libgcrypt 1.8.2

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 4.16.7-1-ARCH

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • Legacy Browser Integration (KeePassHTTP)
  • SSH Agent
  • YubiKey
@droidmonkey droidmonkey self-assigned this May 10, 2018
@droidmonkey droidmonkey changed the title Download favicon throws segmentation fault in 2.3.3 Cancelling favicon download causes a crash May 11, 2018
@droidmonkey
Copy link
Member

I cannot reproduce this. Tried multiple entries, disabled my internet, etc.

@fernandojosecabral
Copy link

fernandojosecabral commented May 13, 2018

Same issue here with the favicon. Just a little bit different: after selecting "Icons" --> "Use custom icon" and then clicking on "Downlad favicon", if the favicon is found it is downloaded and shown. Then, when I click on the "Apply" button, the application crashes.

Once I restarted KeePassXC, the icon is still there. So, then I can apply it and things work OK till I try downloading and applying a new icon again.
Please, note: since I am using a Brazilian translation, I don't know for sure if "Use custom icon" is the label used in English. But, even if is not, this should not be a problem in understanding what I mean.

For debugging:
Linux Mint 18.3
KeePassXC - Versão 2.3.1
Revisão: 2fcaeea
Distribuição: AppImage

Bibliotecas:

  • Qt 5.9.4
  • libgcrypt 1.8.1

Sistema operacional: Linux Mint 18.3
Arquitetura da CPU: x86_64
Kernel: linux 4.4.0-122-generic

Extensões habilitadas:

  • Auto-Type
  • Browser Integration
  • Legacy Browser Integration (KeePassHTTP)
  • SSH Agent
  • YubiKey

Tried the same thing with another version I have (as per bellow) and I've got the same result: KeePassXC crashes after downloading an icon followed by a click on Apply. The newer version is:
KeePassXC - Versão 2.3.3
Revisão: 0a155d8

Bibliotecas:

  • Qt 5.5.1
  • libgcrypt 1.8.1

Sistema operacional: Linux Mint 18.3
Arquitetura da CPU: x86_64
Kernel: linux 4.4.0-122-generic

Extensões habilitadas:

  • Auto-Type
  • Browser Integration
  • Legacy Browser Integration (KeePassHTTP)
  • SSH Agent
  • YubiKey

@fernandojosecabral
Copy link

As to droidmonkey changing the title from Download favicon throws segmentation fault in 2.3.3 to Cancelling favicon download causes a crash, I do think the original title is still in good stand because it describes what I have seen in my machine. But maybe the two problems have the same origin.

@droidmonkey
Copy link
Member

droidmonkey commented May 13, 2018

The downloading of favicons changed drastically between 2.3.1 and 2.3.3. If you are seeing a crash then it is likely somewhere else in the code. The fact that the custom icon was still in your database AFTER the crash means the save after every change kicked in BEFORE the crash occurred. This is likely pointing to the crash being with the entry history (another issue entirely) and totally unrelated to favicon. (See #1732)

@fernandojosecabral
Copy link

fernandojosecabral commented May 13, 2018

Droidmonkey, as you can see above, the problem also occurs with version 2.3.3. At least from the user point of view, the behaviour is the same on both versions. As soon as I click any option (Apply, Cancel or OK), the application crashes immediately. But, no matter what, the favicon is still there when I start KeePassXC again. And, after restarting, I have no problem no matter what option I choose (Apply, Cancel, OR). So, it seems to me, after donwloading and saving the icon, something is left undefined or dangling around. Perhaps some stray pointer or variable.

@cilki
Copy link

cilki commented May 14, 2018

This issue occurs about 25% of the time that I use the "download favicon" feature (2.3.3 Arch Linux x86_64 4.16.8-1). The modal dialog reaches 100% and the new icon appears in the custom icon list, but the dialog does not go away and pressing "cancel" causes KeePassXC to crash.

I'm not sure if this matters, but here's one of the URLs that caused a crash: https://app.getpostman.com.

@tohn
Copy link

tohn commented May 17, 2018

I just had the same issue.
I completed the entry, downloaded the favicon, dialog reaches 100% in small window, I clicked "ok" to close the entry, dialog is still there, I clicked "cancel" -> crash.

URL: https://www.outdooractive.com

KeePassXC - Version 2.3.3
Revision: 0a155d8

Libraries:

  • Qt 5.10.1
  • libgcrypt 1.8.2

Operating system: Arch Linux
CPU architecture: x86_64
Kernel: linux 4.16.8-1-ARCH

@droidmonkey
Copy link
Member

I will deep dive the code on this one this weekend.

@droidmonkey
Copy link
Member

Found where the crash occurs, there is a small chance that the "finished" signal does not hide (close) the progress dialog. This causes a nullptr to be used when you manually close the dialog. Fixed in the PR referenced above.

@ostaszewskik
Copy link

Same happens with me. I\ve noticed it only happens if the option 'Use Google as fallback for downloading website icons' is checked.

KeePassXC - Version 2.3.3
Revision: 0a155d8

Libraries:

  • Qt 5.5.1
  • libgcrypt 1.8.1

Operating system: Ubuntu 16.04.4 LTS
CPU architecture: x86_64
Kernel: linux 4.13.0-45-generic

Enabled extensions:

  • Auto-Type
  • Browser Integration
  • Legacy Browser Integration (KeePassHTTP)
  • SSH Agent
  • YubiKey

@TheZ3ro
Copy link
Contributor

TheZ3ro commented Jun 17, 2018

Closing since this is fixed, will be available in version 2.3.4

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

No branches or pull requests

7 participants