diff --git a/Makefile.am b/Makefile.am index f3212b4a..88c64b30 100644 --- a/Makefile.am +++ b/Makefile.am @@ -309,6 +309,7 @@ EXTRA_DIST= \ tests/08-passphrase-file.sh \ tests/09-explicit-params.sh \ tests/10-check-params.sh \ + tests/12-same-file.sh \ tests/shared_test_functions.sh \ tests/shared_valgrind_functions.sh \ tests/test_scrypt.sh \ diff --git a/main.c b/main.c index 2b05e252..342abef0 100644 --- a/main.c +++ b/main.c @@ -25,12 +25,17 @@ */ #include "platform.h" +#include +#include + #include +#include #include #include #include #include #include +#include #include "getopt.h" #include "humansize.h" @@ -96,6 +101,101 @@ scrypt_mode_info(const char * infilename) return (-1); } +/** + * same_destructive_object(sb_in, sb_out): + * Return non-zero if the two stat results identify storage which scrypt must + * not read and write at the same time. Regular files are identified by + * device/inode; block-device aliases are identified by the underlying device. + * Character devices such as /dev/null remain permitted. + */ +static int +same_destructive_object(const struct stat * sb_in, const struct stat * sb_out) +{ + + if (S_ISREG(sb_in->st_mode) && S_ISREG(sb_out->st_mode)) + return ((sb_in->st_dev == sb_out->st_dev) && + (sb_in->st_ino == sb_out->st_ino)); + + if (S_ISBLK(sb_in->st_mode) && S_ISBLK(sb_out->st_mode)) + return (sb_in->st_rdev == sb_out->st_rdev); + + return (0); +} + +/** + * same_file(infile, outfilename): + * Return non-zero if the already-open ${infile} and the path ${outfilename} + * identify the same destructive storage object. If ${outfilename} is NULL, + * compare against standard output instead. If we cannot tell -- most + * importantly if ${outfilename} does not exist yet -- return zero. + */ +static int +same_file(FILE * infile, const char * outfilename) +{ + struct stat sb_in; + struct stat sb_out; + + /* If we can't stat either file, assume that they're different. */ + if (fstat(fileno(infile), &sb_in)) + return (0); + if (outfilename != NULL) { + if (stat(outfilename, &sb_out)) + return (0); + } else if (fstat(fileno(stdout), &sb_out)) { + return (0); + } + + return (same_destructive_object(&sb_in, &sb_out)); +} + +/** + * open_output(infile, outfilename, outfile): + * Open ${outfilename} for writing without truncating it until the descriptor + * we actually opened has been proved to be different from ${infile}. Return + * 1 if both descriptors identify the same destructive object, -1 on error, + * and 0 + * on success with ${outfile} set. + */ +static int +open_output(FILE * infile, const char * outfilename, FILE ** outfile) +{ + struct stat sb_in; + struct stat sb_out; + int fd; + int saved_errno; + + /* Do not truncate until we have compared the opened descriptor. */ + if ((fd = open(outfilename, O_WRONLY | O_CREAT, 0666)) == -1) + return (-1); + + /* Compare the actual objects behind both open descriptors. */ + if (fstat(fileno(infile), &sb_in)) + goto err0; + if (fstat(fd, &sb_out)) + goto err0; + if (same_destructive_object(&sb_in, &sb_out)) { + (void)close(fd); + return (1); + } + + /* Match fopen(..., "wb") truncation for regular output files. */ + if (S_ISREG(sb_out.st_mode) && ftruncate(fd, 0)) + goto err0; + + /* fdopen does not truncate an already-open descriptor. */ + if ((*outfile = fdopen(fd, "wb")) == NULL) + goto err0; + + /* Success! */ + return (0); + +err0: + saved_errno = errno; + (void)close(fd); + errno = saved_errno; + return (-1); +} + /** * scrypt_mode_enc_dec(params, passphrase_entry, passphrase_arg, dec, verbose, * force_resources, infilename, outfilename): @@ -117,6 +217,7 @@ scrypt_mode_enc_dec(struct scryptenc_params params, FILE * infile; FILE * outfile = stdout; char * passwd; + int openrc; int rc; /* If the input isn't stdin, open the file. */ @@ -129,6 +230,17 @@ scrypt_mode_enc_dec(struct scryptenc_params params, infile = stdin; } + /* + * Refuse obvious same-file aliases before prompting. open_output() + * repeats this check on the descriptor it actually opens, closing the + * path race before any regular output file is truncated. + */ + if (same_file(infile, outfilename)) { + warn0("Input and output files are the same: %s", + outfilename != NULL ? outfilename : "standard output"); + goto err1; + } + /* Get the password. */ if (passphrase_entry_readpass(&passwd, passphrase_entry, passphrase_arg, "Please enter passphrase", @@ -154,10 +266,14 @@ scrypt_mode_enc_dec(struct scryptenc_params params, } } - /* If we have an output filename, open it. */ + /* Bind the destructive same-file check to the output we actually opened. */ if (outfilename != NULL) { - if ((outfile = fopen(outfilename, "wb")) == NULL) { - warnp("Cannot open output file: %s", outfilename); + if ((openrc = open_output(infile, outfilename, &outfile)) != 0) { + if (openrc > 0) + warn0("Input and output files are the same: %s", + outfilename); + else + warnp("Cannot open output file: %s", outfilename); goto err2; } } diff --git a/scrypt.1 b/scrypt.1 index b81bb365..ddd8686b 100644 --- a/scrypt.1 +++ b/scrypt.1 @@ -63,6 +63,21 @@ if specified, or the standard output otherwise. The user will be prompted to enter the passphrase used at encryption time to generate the derived encryption key. .Pp +.Ar outfile +cannot be the same file as +.Ar infile : +.Nm +truncates +.Ar outfile +before it reads +.Ar infile , +so it exits with an error instead of destroying the input. +The same restriction applies when standard output refers to the input file. +This check cannot restore an input which the shell has already truncated +before +.Nm +starts. +.Pp .Nm Cm info provides information about the encryption parameters used for .Ar infile . diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh new file mode 100644 index 00000000..d7edf1e9 --- /dev/null +++ b/tests/12-same-file.sh @@ -0,0 +1,263 @@ +#!/bin/sh + +### Constants +c_valgrind_min=1 +reference_file="${scriptdir}/verify-strings/test_scrypt.good" +encrypted_reference_file="${scriptdir}/verify-strings/test_scrypt_good.enc" +plaintext_target="${s_basename}-plaintext.txt" +encrypted_target="${s_basename}-encrypted.enc" +normal_output="${s_basename}-normal.enc" +stderr_enc="${s_basename}-enc.stderr" +stderr_dec="${s_basename}-dec.stderr" + +scenario_cmd() { + # "scrypt enc" must not write into the file it is reading; opening + # the output file truncates it. + cp "${reference_file}" "${plaintext_target}" + + setup_check "scrypt enc same file" + ( + echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ + enc --passphrase dev:stdin-once \ + "${plaintext_target}" "${plaintext_target}" \ + 2> "${stderr_enc}" + expected_exitcode 1 $? > "${c_exitfile}" + ) + + setup_check "scrypt enc same file error" + grep -q "scrypt: Input and output files are the same" \ + "${stderr_enc}" + echo $? > "${c_exitfile}" + + setup_check "scrypt enc same file leaves input alone" + cmp -s "${plaintext_target}" "${reference_file}" + echo $? > "${c_exitfile}" + + # The same applies to "scrypt dec". + cp "${encrypted_reference_file}" "${encrypted_target}" + + setup_check "scrypt dec same file" + ( + echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ + dec --passphrase dev:stdin-once \ + "${encrypted_target}" "${encrypted_target}" \ + 2> "${stderr_dec}" + expected_exitcode 1 $? > "${c_exitfile}" + ) + + setup_check "scrypt dec same file error" + grep -q "scrypt: Input and output files are the same" \ + "${stderr_dec}" + echo $? > "${c_exitfile}" + + setup_check "scrypt dec same file leaves input alone" + cmp -s "${encrypted_target}" "${encrypted_reference_file}" + echo $? > "${c_exitfile}" + + # Writing to a different file must still work. + setup_check "scrypt enc different file" + echo "${password}" | ${c_valgrind_cmd} "${bindir}/scrypt" \ + enc --passphrase dev:stdin-once \ + "${plaintext_target}" "${normal_output}" + echo $? > "${c_exitfile}" + + # Extended static-alias coverage. This block belongs inside scenario_cmd. + alias_password="${s_basename}-alias-password.txt" + printf '%s\n' "${password}" > "${alias_password}" + for alias_mode in enc dec; do + alias_params="" + alias_reference="${encrypted_reference_file}" + if [ "${alias_mode}" = enc ]; then + alias_params="--logN 10 -r 1 -p 1" + alias_reference="${reference_file}" + fi + for alias_kind in hardlink output-symlink input-symlink stdin dotdot; do + alias_input="${s_basename}-${alias_mode}-${alias_kind}-input" + alias_link="${s_basename}-${alias_mode}-${alias_kind}-link" + alias_stderr="${s_basename}-${alias_mode}-${alias_kind}.stderr" + cp "${alias_reference}" "${alias_input}" + alias_source="${alias_input}" + alias_dest="${alias_link}" + case "${alias_kind}" in + hardlink) + ln "${alias_input}" "${alias_link}" + ;; + output-symlink) + ln -s "${alias_input}" "${alias_link}" + ;; + input-symlink) + ln -s "${alias_input}" "${alias_link}" + alias_source="${alias_link}" + alias_dest="${alias_input}" + ;; + stdin) + alias_source="-" + alias_dest="${alias_input}" + ;; + dotdot) + alias_dir="${s_basename}-${alias_mode}-${alias_kind}-dir" + mkdir "${alias_dir}" + alias_dest="${alias_dir}/../$(basename "${alias_input}")" + ;; + esac + + setup_check "scrypt ${alias_mode} ${alias_kind} rejects alias" + ${c_valgrind_cmd} "${bindir}/scrypt" "${alias_mode}" \ + ${alias_params} --passphrase file:"${alias_password}" \ + "${alias_source}" "${alias_dest}" \ + < "${alias_input}" 2> "${alias_stderr}" + expected_exitcode 1 $? > "${c_exitfile}" + + setup_check "scrypt ${alias_mode} ${alias_kind} error" + grep -q "scrypt: Input and output files are the same" \ + "${alias_stderr}" + echo $? > "${c_exitfile}" + + setup_check "scrypt ${alias_mode} ${alias_kind} preserves input" + cmp -s "${alias_input}" "${alias_reference}" + echo $? > "${c_exitfile}" + done + done + + + # stdout is already open: append redirection must not bypass the guard. + for stdout_mode in enc dec; do + stdout_params="" + stdout_reference="${encrypted_reference_file}" + if [ "${stdout_mode}" = enc ]; then + stdout_params="--logN 10 -r 1 -p 1" + stdout_reference="${reference_file}" + fi + for stdout_source in file stdin; do + stdout_target="${s_basename}-stdout-${stdout_mode}-${stdout_source}" + stdout_stderr="${stdout_target}.stderr" + cp "${stdout_reference}" "${stdout_target}" + stdout_input="${stdout_target}" + if [ "${stdout_source}" = stdin ]; then + stdout_input="-" + fi + + setup_check "scrypt ${stdout_mode} ${stdout_source} stdout alias" + ( + # Bound any accidental writes if this guard regresses. + ulimit -f 128 || exit 1 + ${c_valgrind_cmd} "${bindir}/scrypt" "${stdout_mode}" \ + ${stdout_params} \ + --passphrase file:"${alias_password}" \ + "${stdout_input}" < "${stdout_target}" \ + >> "${stdout_target}" 2> "${stdout_stderr}" + expected_exitcode 1 $? > "${c_exitfile}" + ) + + setup_check "scrypt ${stdout_mode} stdout alias error" + grep -q "scrypt: Input and output files are the same" \ + "${stdout_stderr}" + echo $? > "${c_exitfile}" + + setup_check "scrypt ${stdout_mode} stdout preserves input" + cmp -s "${stdout_target}" "${stdout_reference}" + echo $? > "${c_exitfile}" + done + done + + + # Exercise the race between the early path check and the destructive + # output open. A FIFO passphrase source gives us a deterministic gate: + # once the writer has opened the FIFO, scrypt has already completed the + # early same_file() check and is blocked reading the passphrase. + race_input="${s_basename}-race-input" + race_output="${s_basename}-race-output" + race_passfifo="${s_basename}-race-passphrase" + race_ready="${s_basename}-race-ready" + race_go="${s_basename}-race-go" + race_stderr="${s_basename}-race.stderr" + cp "${reference_file}" "${race_input}" + cp "${reference_file}" "${race_output}" + rm -f "${race_passfifo}" "${race_ready}" "${race_go}" + mkfifo "${race_passfifo}" + + ( + exec 3> "${race_passfifo}" + : > "${race_ready}" + while [ ! -e "${race_go}" ]; do + sleep 1 + done + printf '%s\n' "${password}" >&3 + exec 3>&- + ) & + race_writer_pid=$! + + setup_check "scrypt enc rejects output swapped to input after precheck" + ${c_valgrind_cmd} "${bindir}/scrypt" enc --logN 10 -r 1 -p 1 \ + --passphrase file:"${race_passfifo}" \ + "${race_input}" "${race_output}" 2> "${race_stderr}" & + race_scrypt_pid=$! + + race_wait_ready() { + [ ! -e "${race_ready}" ] + } + if wait_while 5000 race_wait_ready; then + rm -f "${race_output}" + ln "${race_input}" "${race_output}" + : > "${race_go}" + + wait "${race_scrypt_pid}" + race_rc=$? + wait "${race_writer_pid}" + race_writer_rc=$? + if [ "${race_writer_rc}" -ne 0 ]; then + race_rc=1 + fi + expected_exitcode 1 "${race_rc}" > "${c_exitfile}" + else + : > "${race_go}" + kill "${race_scrypt_pid}" 2>/dev/null || true + kill "${race_writer_pid}" 2>/dev/null || true + wait "${race_scrypt_pid}" 2>/dev/null || true + wait "${race_writer_pid}" 2>/dev/null || true + echo 1 > "${c_exitfile}" + fi + + setup_check "scrypt enc path-swap race reports same file" + grep -q "scrypt: Input and output files are the same" \ + "${race_stderr}" + echo $? > "${c_exitfile}" + + setup_check "scrypt enc path-swap race preserves input" + cmp -s "${race_input}" "${reference_file}" + echo $? > "${c_exitfile}" + + rm -f "${race_passfifo}" "${race_ready}" "${race_go}" + + # A character device is not a destructive same-regular-file alias. + setup_check "scrypt enc permits same null device" + ${c_valgrind_cmd} "${bindir}/scrypt" enc --logN 10 -r 1 -p 1 \ + --passphrase file:"${alias_password}" /dev/null /dev/null + echo $? > "${c_exitfile}" + + # Equal contents are not the same inode; preserve and roundtrip the input. + independent_input="${s_basename}-independent-input" + independent_output="${s_basename}-independent-output" + independent_plain="${s_basename}-independent-plain" + cp "${reference_file}" "${independent_input}" + cp "${reference_file}" "${independent_output}" + setup_check "scrypt enc permits identical contents in independent files" + ${c_valgrind_cmd} "${bindir}/scrypt" enc --logN 10 -r 1 -p 1 \ + --passphrase file:"${alias_password}" \ + "${independent_input}" "${independent_output}" + echo $? > "${c_exitfile}" + + setup_check "scrypt enc independent files preserves input" + cmp -s "${independent_input}" "${reference_file}" + echo $? > "${c_exitfile}" + + setup_check "scrypt dec independent output roundtrip" + ${c_valgrind_cmd} "${bindir}/scrypt" dec \ + --passphrase file:"${alias_password}" \ + "${independent_output}" "${independent_plain}" + echo $? > "${c_exitfile}" + + setup_check "scrypt dec independent output matches input" + cmp -s "${independent_plain}" "${reference_file}" + echo $? > "${c_exitfile}" +}