keepassxreboot / keepassxc

KeePassXC is a cross-platform community-driven port of the Windows application “Keepass Password Safe”.
https://keepassxc.org/
Other
20.07k stars 1.42k forks source link

Remote sync doesn't verify downloaded file #10910

Open hifi opened 2 weeks ago

hifi commented 2 weeks ago

Overview

At least when using the "Test" button the test is successful if the command succeeds and creates either a file or a directory.

Steps to Reproduce

  1. Make download command create a directory to {TEMP_DATABASE}
  2. Test is successful

Expected Behavior

The file must exist and have a KDBX header, no other result should be accepted. Testing against an empty remote will fail but that might be good as long as the error (stdout + stderr) is displayed.

Actual Behavior

An empty directory is accepted as a successful download or any file that isn't necessarily a database.

droidmonkey commented 2 weeks ago

@t-h-e

t-h-e commented 2 weeks ago

Adding the check is easy. Can anyone tell me how to check the header though? Is there a method that does check for a kdbx header already?

hifi commented 2 weeks ago

KeePass2Reader::readDatabase reads the signature before reading the database. A small refactor to split that functionality out to a different method would allow using KeePass2Reader to just verify the signature and get the error if it fails.

The test could also just show a different success message if the command succeeds but no file is downloaded to indicate no errors happened but nothing was downloaded either.

droidmonkey commented 2 weeks ago

We have a method to only read the public headers. That'll suffice.

https://github.com/keepassxreboot/keepassxc/blob/develop/src%2Fgui%2FDatabaseOpenWidget.cpp#L228

t-h-e commented 2 weeks ago

@droidmonkey thanks. I have added a check. @hifi can you have a look?

See https://github.com/keepassxreboot/keepassxc/pull/10915