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..378112cc 100644 --- a/tests/08-passphrase-file.sh +++ b/tests/08-passphrase-file.sh @@ -10,6 +10,14 @@ 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" +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" scenario_cmd() { # Create the passphrase file. @@ -32,7 +40,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}" @@ -44,7 +52,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. @@ -53,7 +61,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}" @@ -66,4 +74,57 @@ 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}" + + # 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}" + ${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}" }