Skip to content

[bug bounty] readpass_file() silently truncates the passphrase at a carriage return #431

Description

@woahwhattheheck

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

Summary

readpass_file() silently truncates the passphrase at the first carriage
return, so --passphrase file:FILENAME can encrypt or decrypt with a
passphrase which is not the one the file contains.

Its documented contract in libcperciva/util/readpass.h says such a file
should be rejected:

Print an error and fail if the file is 2048 characters or more, or if it
contains any newline \n or \r\n characters other than at the end of the
file
. Do not include the \n or \r\n characters in the passphrase.

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

libcperciva/util/readpass_file.c:

	/* Get a line from the file. */
	if ((fgets(passbuf, MAXPASSLEN, f)) == NULL) {
...
	/* Bail if there's the line is too long, or if there's a second line. */
	if (fgetc(f) != EOF) {
...
	/* Truncate at any newline character. */
	passbuf[strcspn(passbuf, "\r\n")] = '\0';

fgets() stops after a \n, so the "more than 1 line" check catches a stray
\n: any \n we hold is necessarily the last character. A \r is different
fgets() reads straight past it — so a \r anywhere in the line reaches
strcspn(), which cuts the passphrase off at that point without a word.

Reproducer

The passphrase file below begins with the correct passphrase, so the
truncation is invisible: the decryption succeeds using a passphrase which is
not what the file holds.

$ printf 'hunter2\rextra\n' > pw.txt
$ scrypt dec --passphrase file:pw.txt reference.enc out.txt
$ echo $?
0

reference.enc here is tests/verify-strings/test_scrypt_good.enc, which was
encrypted with hunter2. The file says the passphrase is hunter2\rextra;
scrypt used hunter2.

The same applies when encrypting, and because the truncation is applied
identically both ways it round-trips, so nothing reveals it until the
passphrase is entered by some other route — typed at the prompt, or via
--passphrase env:VAR — and then does not work.

A passphrase file whose only newline is a \n or a trailing \r\n is
unaffected, so the ordinary Unix and Windows cases are both fine today.

Fix

PR to follow: strip a trailing \n or \r\n explicitly, then reject the file
if any \r or \n remains, which is what the header already promises.
Cutting the passphrase short and keeping the character as passphrase data both
change the passphrase an existing file yields, so neither is safe to pick
silently.

It extends tests/08-passphrase-file.sh rather than adding a new test file.

How this was found

Source review of libcperciva/util/readpass_file.c at a71ae82 against the
contract in libcperciva/util/readpass.h.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions