Skip to content

fix: an export password of "0" is a password - #863

Merged
blaipr merged 1 commit into
mainfrom
fix/an-export-password-of-zero-is-a-password
Aug 23, 2026
Merged

fix: an export password of "0" is a password#863
blaipr merged 1 commit into
mainfrom
fix/an-export-password-of-zero-is-a-password

Conversation

@blaipr

@blaipr blaipr commented Aug 23, 2026

Copy link
Copy Markdown
Member

Whether the admin had supplied a password to encrypt an export with was decided by
empty(), in four places. empty('0') is true, so a password of exactly "0" was treated
everywhere as no password at all:

  • appendNode() wrote every category, client, tag and account into the file in the clear.
  • appendHash() signed the integrity hash with sha1(passwordSalt) instead of the password.
  • XmlVerify::verify() then read the file back as unencrypted and agreed with it, so the
    self-check that runs immediately after the export reported success.
  • The web form's "passwords do not match" check was skipped, so the value was never even
    compared against the one typed to confirm it.

The result is an export the admin believes is encrypted, containing every account's
name, login, URL and notes, sitting in a file they will reasonably then email or copy
somewhere — with nothing anywhere reporting that the password was dropped.

The question being asked is "did the admin supply a password", and null and the empty
string are the two answers that mean no — an unencrypted export is a supported thing to
ask for. That is now settled once at each entry point, so the decisions below it compare
against null rather than each re-deciding what counts as absent.

Checked by putting empty() back: the export written with "0" is no longer encrypted and
the new test fails. The companion test, that an empty password still writes in the clear,
is there so that a fix which simply encrypted unconditionally would not satisfy it.

The two new tests also needed PHPUnit\Framework\Attributes\Test importing into that file
— it was not there, the rest of the class uses the test-prefixed naming, and without it
#[Test] does not resolve and both new methods were collected as nothing at all. They
ran green that way before the import was added.

Whether the admin had supplied a password to encrypt an export with was decided by
empty(), in four places. empty('0') is true, so a password of exactly "0" was treated
everywhere as no password at all:

- appendNode() wrote every category, client, tag and account into the file in the clear.
- appendHash() signed the integrity hash with sha1(passwordSalt) instead of the password.
- XmlVerify::verify() then read the file back as unencrypted and agreed with it, so the
  self-check that runs immediately after the export reported success.
- The web form's "passwords do not match" check was skipped, so the value was never even
  compared against the one typed to confirm it.

The result is an export the admin believes is encrypted, containing every account's
name, login, URL and notes, sitting in a file they will reasonably then email or copy
somewhere — with nothing anywhere reporting that the password was dropped.

The question being asked is "did the admin supply a password", and null and the empty
string are the two answers that mean no — an unencrypted export is a supported thing to
ask for. That is now settled once at each entry point, so the decisions below it compare
against null rather than each re-deciding what counts as absent.

Checked by putting empty() back: the export written with "0" is no longer encrypted and
the new test fails. The companion test, that an empty password still writes in the clear,
is there so that a fix which simply encrypted unconditionally would not satisfy it.

The two new tests also needed PHPUnit\Framework\Attributes\Test importing into that file
— it was not there, the rest of the class uses the test-prefixed naming, and without it
`#[Test]` does not resolve and both new methods were collected as nothing at all. They
ran green that way before the import was added.
@blaipr
blaipr merged commit 941110f into main Aug 23, 2026
8 checks passed
@blaipr
blaipr deleted the fix/an-export-password-of-zero-is-a-password branch August 23, 2026 22:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant