Kunzisoft / KeePassDX

Lightweight vault and password manager for Android, KeePassDX allows editing encrypted data in a single file in KeePass format and fill in the forms in a secure way.
https://www.keepassdx.com/
GNU General Public License v3.0
4.78k stars 276 forks source link

Database deletion when phone restarts while waiting for token #1832

Closed nicocalu closed 6 months ago

nicocalu commented 6 months ago

Describe the bug

So this has happened to me a few times before, and so I decided to investigate today the issue: When I open my database (with a yubico token), and then copy some passwords and stuff, and accidentally click on something that triggers a save (like the save data icon up top when having an entry open), the dialogue for presenting the token appears. Often, as I know I haven't made a modification that I want to save, i just exit the app. Now, my phone normally restarts at night for updates (not every day but it happens sometimes), and when i wake up the next morning, my database is gone.

Well, not really gone, but empty (0 bytes)

Sadly, in the logs nothing appears, but the issue seems to be very repeatable.

To Reproduce

Steps to reproduce the behavior: [restart phone if you want a clean slate]

  1. Create a new database (if you don't want to delete an existing one) (using a yubico to encrypt it)
  2. Add an entry to it
  3. Save the database
  4. Close it
  5. Open it again
  6. Do a thing that triggers a save (and so the yubico driver screen appears)
  7. Restart the phone
  8. Voila!

Expected behavior

The database should get saved (without the modifications), instead of getting deleted.

KeePass Database

KeePassDX:

Android:

Additional context

Using Key Driver version 0.1.8

J-Jamet commented 6 months ago

Duplicate https://github.com/Kunzisoft/KeePassDX/issues/1680 Issue solved in develop branch

nicocalu commented 5 months ago

Can i just ask why has this issue been reported since November of last year, and just today this bug deleted all of my database? Where do I get the new update that has the fix? I don't want to be the the entitled nobody that demands stuff from open source developers, but ; this issue deletes all of your database! I think that is enough reason for an urgent patch 😢

J-Jamet commented 5 months ago

I'm aware of this, and I've made a patch, but it's more of a workaround. The real problem comes from the exception raising by the file manager after it has indicated that the file is OK to write. I had already tried to solve the problem with a temp cache, but the file was still initialized during the exception, creating an empty file that received no data. I made sure not to initialize the file at all in the stream and keep the temp cache, after which the exception will still be there and the behavior will depend on the handler used, but in any case there will be no modified data at all. I wanted to reproduce the problem and see if it didn't lead to others, and it looks good, so I'll make a version 4.0.7 when I can.