Skip to content

main: refuse to write the output into the input file - #428

Open
woahwhattheheck wants to merge 3 commits into
Tarsnap:masterfrom
woahwhattheheck:scrypt-refuse-same-file
Open

main: refuse to write the output into the input file#428
woahwhattheheck wants to merge 3 commits into
Tarsnap:masterfrom
woahwhattheheck:scrypt-refuse-same-file

Conversation

@woahwhattheheck

@woahwhattheheck woahwhattheheck commented Sep 6, 2026

Copy link
Copy Markdown

Disclosure, per this repository's AGENTS.md: I am an LLM (Claude), submitting on behalf of the account owner. I am available to discuss this change and to revise it in response to review feedback.

Fixes #427.

scrypt enc secret.txt secret.txt opens secret.txt for reading, reads a
passphrase, then opens the same path with fopen(..., "wb") — which truncates
it. Nothing has been read from the input yet, so the first fread() in
scryptenc_file() returns 0, ferror() is false, and the empty read is
indistinguishable from a legitimately empty file. scryptenc_file() writes the
header and HMAC, returns SCRYPT_OK, and scrypt exits 0. The user's file is
replaced by a 128-byte encryption of nothing, silently.

scrypt dec f f loses the ciphertext the same way, and both are reachable with
the input on stdin (scrypt enc - f < f).

The change

same_file() compares the already-open input against the output path with
fstat()/stat() and reports whether they are the same regular file:

  • It uses fstat(fileno(infile)) rather than stat(infilename) so that the
    stdin cases are covered too, and so that symlinks, hard links and different
    spellings of the same path are all handled.
  • Only regular files count — opening a character device such as /dev/null
    for writing discards nothing, so scrypt enc /dev/null /dev/null keeps
    working.
  • If either stat fails the answer is "different", so the normal case where
    the output file does not exist yet costs one failed stat() and nothing
    else.

The check sits immediately after the input is opened and before the
passphrase is read, so the user is not asked to type a passphrase for a command
that cannot succeed. It jumps to the existing err1 label, which closes the
input; no output file is created.

fileno() is already used in libcperciva/util/readpass.c and <sys/types.h>
in lib-platform/util/memlimit.c, so this adds no new portability burden.

Also

  • tests/12-same-file.sh covers enc and dec with the same file, asserts
    exit status 1, the error message, and that the input is byte-for-byte
    unchanged; plus a check that encrypting to a different file still works, so
    a false positive in same_file() would be caught.
  • scrypt.1 documents the restriction.

Note on the test number

The test is numbered 12 rather than 11 so that it does not collide with
tests/11-info.sh from #426. The ordering works whichever of the two lands
first; the only overlap is the adjacent line each adds to EXTRA_DIST in
Makefile.am. Happy to renumber if you would rather not take #426.

"scrypt enc secret.txt secret.txt" opens secret.txt for reading, reads a
passphrase, then opens the same path with fopen(..., "wb"), which truncates
it.  scryptenc_file() then reads from the now-empty file, gets EOF on the
first fread(), writes a valid 128-byte scrypt file containing an encryption
of no data, and returns SCRYPT_OK; scrypt exits 0.  The file the user asked
to encrypt is gone, with no error and nothing to indicate that anything went
wrong.

"scrypt dec f f" destroys the ciphertext the same way, and both are also
reachable with the input on stdin, e.g. "scrypt enc - f < f".

Compare the open input file and the output path with fstat() and stat()
before opening the output, and exit with an error if they are the same
regular file.  Devices are excluded: opening /dev/null for writing discards
nothing.  The check runs before the passphrase is read, so the user is not
asked to type a passphrase for a command which cannot succeed.

Add tests/12-same-file.sh, and document the restriction in scrypt.1.
The previous commit was uploaded through the GitHub API by a helper
which dropped the final newline of every text file it sent.  Rewrite
those files with their trailing newline intact; no other change.
Add the authored static-alias and independent-file regression coverage to the existing test scenario. Preserve the application implementation and existing upstream submission.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

[bug bounty] "scrypt enc f f" silently destroys f: the output file is truncated before the input is read

2 participants