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

Summary

scrypt enc secret.txt secret.txt destroys secret.txt and exits 0. The file
is replaced by a 128-byte scrypt file whose plaintext is empty; the original
contents are gone, and nothing is printed to say so.

This is not a security issue, so I am reporting it here rather than by email,
per https://www.tarsnap.com/bugbounty.html.

What happens

scrypt_mode_enc_dec() in main.c opens the input, reads the passphrase, and
only then opens the output:

	/* If the input isn't stdin, open the file. */
	if (infilename != NULL) {
		if ((infile = fopen(infilename, "rb")) == NULL) {
...
	/* If we have an output filename, open it. */
	if (outfilename != NULL) {
		if ((outfile = fopen(outfilename, "wb")) == NULL) {

When the two names refer to the same file, fopen(outfilename, "wb")
truncates it to zero length. Nothing has been read from infile yet — the
first fread() in scryptenc_file() happens afterwards — so that fread()
returns 0 immediately:

	do {
		if ((readlen = fread(buf, 1, ENCBLOCK, infile)) == 0)
			break;

ferror(infile) is false, so this is indistinguishable from a legitimately
empty input. scryptenc_file() writes the 96-byte header and the 32-byte
HMAC, returns SCRYPT_OK, and scrypt exits 0.

Reproducer

$ echo "important data" > secret.txt
$ echo "hunter2" | scrypt enc --passphrase dev:stdin-once secret.txt secret.txt
$ echo $?
0
$ ls -l secret.txt
-rw-r--r--  1 user  user  128 ... secret.txt
$ echo "hunter2" | scrypt dec --passphrase dev:stdin-once secret.txt
$ echo $?
0

The final decryption succeeds and prints nothing: the file now correctly
contains an encryption of zero bytes. The original 15 bytes are unrecoverable.

scrypt dec f f loses the ciphertext the same way — the header is read into
the cookie by scryptdec_file_prep(), then the file is truncated, then
scryptdec_file_copy() fails on the empty remainder — leaving f as a
zero-length file.

Both are also reachable with the input on stdin:

echo "hunter2" | scrypt enc --passphrase dev:stdin-once - f < f

Why I think this is worth fixing rather than "don't do that"

scrypt opens both files itself; the shell is not involved, so this is not the
sort f > f situation where the shell truncates before the program runs. A
user asking to encrypt a file in place is a natural thing to try, scrypt(1)
does not say it is unsupported, and the failure is silent and total —
cp, mv and install all diagnose the same mistake instead.

Fix

PR to follow: 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, since opening /dev/null for writing
discards nothing. The check runs before the passphrase is read, so the user is
not prompted for a passphrase that cannot be used.

It adds tests/12-same-file.sh and a note in scrypt.1.

How this was found

Source review of main.c and lib/scryptenc/scryptenc.c at a71ae82,
following the ordering of the fopen() calls against the first fread().