From 3fcb6826f01157682038036fcea2620b3509a12c Mon Sep 17 00:00:00 2001 From: tokenjunkielabs Date: Sun, 6 Sep 2026 12:33:19 +0000 Subject: [PATCH 1/3] readpass_file: reject a passphrase file with a stray carriage return readpass_file() finished with passbuf[strcspn(passbuf, "\r\n")] = '\0'; fgets() stops after a "\n", and the fgetc() check above rejects anything which follows it, so a "\n" in the buffer is always the last character. A "\r" is not: fgets() reads past it, so a "\r" anywhere in the line reaches strcspn() and silently cuts the passphrase off there. "--passphrase file:pw" with pw holding "hunter2\rextra" therefore used "hunter2". It round-trips, since encryption truncates the same way, so nothing showed until the passphrase was supplied by another route. Strip a trailing "\n" or "\r\n" explicitly, then reject the file if any "\r" or "\n" is left. Keeping the character as passphrase data would also change the passphrase an existing file yields, so neither answer is safe to pick silently. readpass.h already documented this behaviour. Extend tests/08-passphrase-file.sh: a file holding "hunter2\rextra" must be rejected -- it begins with the correct passphrase, so before this change that decryption succeeded -- and a CRLF file must still work. --- libcperciva/util/readpass_file.c | 25 ++++++++++++++++++-- tests/08-passphrase-file.sh | 39 ++++++++++++++++++++++++++++++++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/libcperciva/util/readpass_file.c b/libcperciva/util/readpass_file.c index 4379a61d..1f0cd35f 100644 --- a/libcperciva/util/readpass_file.c +++ b/libcperciva/util/readpass_file.c @@ -22,6 +22,7 @@ readpass_file(char ** passwd, const char * filename) { FILE * f; char passbuf[MAXPASSLEN]; + size_t len; /* Open the file. */ if ((f = fopen(filename, "r")) == NULL) { @@ -52,8 +53,28 @@ readpass_file(char ** passwd, const char * filename) goto err1; } - /* Truncate at any newline character. */ - passbuf[strcspn(passbuf, "\r\n")] = '\0'; + /* + * Strip a trailing "\n" or "\r\n". fgets() stops after a "\n", so + * if the file contains one then it is the last character we read. + */ + len = strlen(passbuf); + if ((len > 0) && (passbuf[len - 1] == '\n')) { + passbuf[--len] = '\0'; + if ((len > 0) && (passbuf[len - 1] == '\r')) + passbuf[--len] = '\0'; + } + + /* + * Any "\r" or "\n" left is not a line ending we strip. We could cut + * the passphrase short there, or keep the character as part of the + * passphrase, but both change the passphrase which an existing file + * yields, and neither is safe to pick silently. Refuse the file. + */ + if (strcspn(passbuf, "\r\n") != len) { + warn0("passphrase in %s contains a carriage return or newline", + filename); + goto err1; + } /* Copy the password out. */ if ((*passwd = strdup(passbuf)) == NULL) { diff --git a/tests/08-passphrase-file.sh b/tests/08-passphrase-file.sh index 1d1de088..ff4df75f 100644 --- a/tests/08-passphrase-file.sh +++ b/tests/08-passphrase-file.sh @@ -10,6 +10,11 @@ decrypted_reference_file="${s_basename}-attempt_reference.txt" decrypted_badpass_file="${s_basename}-decrypt-badpass.txt" decrypted_badpass_log="${s_basename}-decrypt-badpass.log" decrypted_no_file_log="${s_basename}-decrypt-no-file.log" +cr_passphrase_file="${s_basename}-passphrase-cr.txt" +decrypted_cr_file="${s_basename}-decrypt-cr.txt" +decrypted_cr_log="${s_basename}-decrypt-cr.log" +crlf_passphrase_file="${s_basename}-passphrase-crlf.txt" +decrypted_crlf_file="${s_basename}-decrypt-crlf.txt" scenario_cmd() { # Create the passphrase file. @@ -66,4 +71,38 @@ scenario_cmd() { setup_check "scrypt dec file bad no file" test -e "${decrypted_badpass_file}" expected_exitcode 1 $? > "${c_exitfile}" + + # A passphrase file containing a carriage return which is not part of + # a trailing CRLF must be rejected. The passphrase used here begins + # with the correct passphrase, so accepting it would decrypt the file + # with a passphrase which is not the one the file contains. + setup_check "scrypt dec file stray CR" + printf '%s\rextra\n' "${password}" > "${cr_passphrase_file}" + ${c_valgrind_cmd} "${bindir}/scrypt" \ + dec --passphrase file:"${cr_passphrase_file}" \ + "${encrypted_reference_file}" "${decrypted_cr_file}" \ + 2> "${decrypted_cr_log}" + expected_exitcode 1 $? > "${c_exitfile}" + + # We should have received an error message. + setup_check "scrypt dec file stray CR error" + grep -q "carriage return or newline" "${decrypted_cr_log}" + echo "$?" > "${c_exitfile}" + + # We should not have created a file. + setup_check "scrypt dec file stray CR no file" + test -e "${decrypted_cr_file}" + expected_exitcode 1 $? > "${c_exitfile}" + + # A passphrase file with CRLF line endings must still work. + setup_check "scrypt dec file CRLF" + printf '%s\r\n' "${password}" > "${crlf_passphrase_file}" + ${c_valgrind_cmd} "${bindir}/scrypt" \ + dec --passphrase file:"${crlf_passphrase_file}" \ + "${encrypted_reference_file}" "${decrypted_crlf_file}" + echo $? > "${c_exitfile}" + + setup_check "scrypt dec file CRLF output against reference" + cmp -s "${decrypted_crlf_file}" "${reference_file}" + echo $? > "${c_exitfile}" } From 1cb6ecf8269e0a37631e6c7dc6e63e05133c4b8e Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Tue, 8 Sep 2026 09:19:44 -0400 Subject: [PATCH 2/3] Align passphrase failure assertions with their output paths (#2) Use distinct output paths for the missing-file and incorrect-passphrase cases, and check the path used by each command. Preserve the existing implementation and upstream submission. --- tests/08-passphrase-file.sh | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tests/08-passphrase-file.sh b/tests/08-passphrase-file.sh index ff4df75f..44ce960a 100644 --- a/tests/08-passphrase-file.sh +++ b/tests/08-passphrase-file.sh @@ -10,6 +10,7 @@ decrypted_reference_file="${s_basename}-attempt_reference.txt" decrypted_badpass_file="${s_basename}-decrypt-badpass.txt" decrypted_badpass_log="${s_basename}-decrypt-badpass.log" decrypted_no_file_log="${s_basename}-decrypt-no-file.log" +decrypted_no_file="${s_basename}-decrypt-no-file.txt" cr_passphrase_file="${s_basename}-passphrase-cr.txt" decrypted_cr_file="${s_basename}-decrypt-cr.txt" decrypted_cr_log="${s_basename}-decrypt-cr.log" @@ -37,7 +38,7 @@ scenario_cmd() { setup_check "scrypt dec file none" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase file:THIS_FILE_DOES_NOT_EXIST \ - "${encrypted_reference_file}" "${decrypted_reference_file}" \ + "${encrypted_reference_file}" "${decrypted_no_file}" \ 2> "${decrypted_no_file_log}" expected_exitcode 1 $? > "${c_exitfile}" @@ -49,7 +50,7 @@ scenario_cmd() { # We should not have created a file. setup_check "scrypt dec file none no file" - test -e "${decrypted_badpass_file}" + test -e "${decrypted_no_file}" expected_exitcode 1 $? > "${c_exitfile}" # Attempt to decrypt the reference file with an incorrect passphrase. @@ -58,7 +59,7 @@ scenario_cmd() { echo "bad-pass" > "${bad_passphrase_file}" ${c_valgrind_cmd} "${bindir}/scrypt" \ dec --passphrase file:"${bad_passphrase_file}" \ - "${encrypted_reference_file}" "${decrypted_reference_file}" \ + "${encrypted_reference_file}" "${decrypted_badpass_file}" \ 2> "${decrypted_badpass_log}" expected_exitcode 1 $? > "${c_exitfile}" From 5a08e1f03294cc8edf2446961d59cfebe2c0819b Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 19 Sep 2026 22:36:27 -0400 Subject: [PATCH 3/3] tests: cover stray CR passphrase on encryption --- tests/08-passphrase-file.sh | 21 +++++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/tests/08-passphrase-file.sh b/tests/08-passphrase-file.sh index 44ce960a..378112cc 100644 --- a/tests/08-passphrase-file.sh +++ b/tests/08-passphrase-file.sh @@ -14,6 +14,8 @@ decrypted_no_file="${s_basename}-decrypt-no-file.txt" cr_passphrase_file="${s_basename}-passphrase-cr.txt" decrypted_cr_file="${s_basename}-decrypt-cr.txt" decrypted_cr_log="${s_basename}-decrypt-cr.log" +encrypted_cr_file="${s_basename}-encrypt-cr.enc" +encrypted_cr_log="${s_basename}-encrypt-cr.log" crlf_passphrase_file="${s_basename}-passphrase-crlf.txt" decrypted_crlf_file="${s_basename}-decrypt-crlf.txt" @@ -95,6 +97,25 @@ scenario_cmd() { test -e "${decrypted_cr_file}" expected_exitcode 1 $? > "${c_exitfile}" + # Encryption must reject the same malformed passphrase file rather than + # silently encrypting with the prefix before the carriage return. + setup_check "scrypt enc file stray CR" + ${c_valgrind_cmd} "${bindir}/scrypt" \ + enc --passphrase file:"${cr_passphrase_file}" \ + "${reference_file}" "${encrypted_cr_file}" \ + 2> "${encrypted_cr_log}" + expected_exitcode 1 $? > "${c_exitfile}" + + # We should have received the same malformed-passphrase diagnostic. + setup_check "scrypt enc file stray CR error" + grep -q "carriage return or newline" "${encrypted_cr_log}" + echo "$?" > "${c_exitfile}" + + # We should not have created an encrypted output file. + setup_check "scrypt enc file stray CR no file" + test -e "${encrypted_cr_file}" + expected_exitcode 1 $? > "${c_exitfile}" + # A passphrase file with CRLF line endings must still work. setup_check "scrypt dec file CRLF" printf '%s\r\n' "${password}" > "${crlf_passphrase_file}"