[PRE-RELEASE] Crash when opening database with corrupt favicons

[whisdol]

Expected Behavior

Database should open

Current Behavior

KeePassXC crashes when attempting to open the database. In build 8bfc539, the crash is silent. In the snapshot from January 13th, an "unknown software exception" (0x40000015) is thrown at 0x0000000066A1381B.

Possible Solution

As the file was corrupted by the current stable version (2.3.4), KeePassXC should clean up the favicon list. If that fails, clear all manually added favicons and replace them with the standard icon.

Steps to Reproduce

1. Add manual favicons to a database with KeePassXC 2.3.4 (in a buggy way, see below).
2. Open the same database with the latest 2.4.0 snapshot.

Context

I was trying to prettify my database with KeePassXC 2.3.4 by downloading/manually adding favicons to the entries.
As the automatic download did not work for the few entries I tried, I manually downloaded .icos and imported them with the "user defined symbol" option. There, I encountered some off-by-one bug, as the new icon did not show up correctly in the list of symbols. It only showed up after adding another icon (which in turn did not show up until I added another icon, ... repeat).
KeePassXC 2.3.4 has no problem handling my database. However, the latest snapshot crashes when opening this database. 8bfc539 works fine if I delete all manually added symbols from the database file.
I cleared my database of all real data and I'm still able to reproduce it with this file. As I'm not sure how to verify that no data is recoverable, I don't feel comfortable sharing it as a public attachment here, but I could probably send it to someone via mail (or similar).
I am unable to reproduce the initial off-by-one bug in the user defined symbols list with a new database.

Debug Info

KeePassXC - Version 2.4.0-snapshot
Build Type: Snapshot
Revision: 8bfc539

Bibliotheken:

• Qt 5.12.0
• libgcrypt 1.8.4

Betriebssystem: Windows 10 (10.0)
CPU-Architektur: x86_64
Kernel: winnt 10.0.17134

Aktivierte Erweiterungen:

• Auto-Type
• Browser Integration
• SSH Agent
• KeeShare (only unsigned sharing)
• YubiKey

[droidmonkey]

You can send the database and password to [email protected]. if you use PGP, I encourage you to encrypt the email as well.

Alternatively, try to create a brand new database with the glitchy icons and post it here.

[droidmonkey]

The "crash" was actually an assert hit because there were duplicate icon UUID's in the database. I removed the assert and cleaned up how we handle duplicate UUIDs. This allows the database to open and fixes the corruption upon subsequent save. The downside to this is the corrupt icon will be replaced with whatever its duplicate was on entries. A minor inconvenience, but the corruption is fixed for future loads.

[whisdol]

I am now testing 2.4.0-beta2 (on Windows 10) and I believe I just experienced the same crash after downloading a favicon with the following "crash message":

(The instruction in ... pointed to ... in RAM. The operation read could not be completed in RAM.)
I was going through my entries and downloading the favicons one after the other before the crash occurred. Auto-save on change is enabled, in case that is relevant.
The database could be opened after the crash (thanks for the fix :)). In the custom icon list, the newly downloaded icon appeared - but it was not assigned to the entry (I guess this is expected). Downloading the same favicon again was possible without a crash - I now have it twice in the list of custom icons.
Maybe there is still an underlying issue with assigning a new UUID for downloaded favicons?

[droidmonkey]

You likely triggered the entry saving crash...