IJHack / QtPass / 35635087026

93%

Build Type

push

github

Committed by web-flow

Commit Message

Insert: gpg never opens a path in the store for output (#1882)

* Insert: gpg never opens a path in the store for output

ImitatePass::Insert() ran gpg with --output on the entry's own path right
after refuseLinkedPath() had judged the name. Whoever can write to the
store could make the name a link in between, and gpg --yes wrote the
ciphertext through to whatever the link pointed at (reproduced: a file
outside the store overwritten with ciphertext). The link checks cannot
close that window; not opening store paths by name can.

gpg encrypts into a QTemporaryDir of QtPass's own (0700, outside the
store, out of a co-writer's reach). When its step ends, before the git
steps queued behind it run, placeEncryptedFile() reads that output and
writes it through a QTemporaryFile created next to the entry, by its open
handle, then gives it the entry's name with Util::replaceFile(): rename(2)
or MoveFileEx, which replaces whatever entry is under the name (a planted
link as an entry) and follows nothing. Not QFile::rename(): when the plain
rename fails it falls back to a copy that opens the target by name, and
would write through a link planted in between. Without overwrite, link()
and unlink refuse a name that gained an entry since the check. If the
temporary's own name was swapped for a link, the entry is now that link
and the insert is reported as failed; nothing was written through it.

A failed placement is not shown from inside the executor's completion (a
dialog would spin the event loop and let the queued git steps run first):
finished() turns it into a failed gpg step, the cancel loop drops the git
steps, and the reason reaches the interface through processErrorExit. A
store-relative name, which the dialog passes for a new entry, is resolved
against the store first: gpg used to do that in its working directory.
Entries are written 0600, as pass writes them.

Tests (tst_imitatepass): gpg's --output is outside the store and the
scratch is gone afterwards, ... (continued)

Coverage Stats

45 of 63 new or added lines in 2 files covered. (71.43%)

8 existing lines in 2 files now uncovered.

7419 of 9390 relevant lines covered (79.01%)

80.39 hits per line