From 70512977e19f9767af3fc68466f3518bff0ca89c Mon Sep 17 00:00:00 2001 From: tokenjunkielabs Date: Sun, 6 Sep 2026 11:22:04 +0000 Subject: [PATCH 1/8] main: refuse to write the output into the input file "scrypt enc secret.txt secret.txt" opens secret.txt for reading, reads a passphrase, then opens the same path with fopen(..., "wb"), which truncates it. scryptenc_file() then reads from the now-empty file, gets EOF on the first fread(), writes a valid 128-byte scrypt file containing an encryption of no data, and returns SCRYPT_OK; scrypt exits 0. The file the user asked to encrypt is gone, with no error and nothing to indicate that anything went wrong. "scrypt dec f f" destroys the ciphertext the same way, and both are also reachable with the input on stdin, e.g. "scrypt enc - f < f". 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: opening /dev/null for writing discards nothing. The check runs before the passphrase is read, so the user is not asked to type a passphrase for a command which cannot succeed. Add tests/12-same-file.sh, and document the restriction in scrypt.1. --- Makefile.am | 3 ++- main.c | 47 +++++++++++++++++++++++++++++++- scrypt.1 | 12 ++++++++- tests/12-same-file.sh | 63 +++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 122 insertions(+), 3 deletions(-) create mode 100644 tests/12-same-file.sh diff --git a/Makefile.am b/Makefile.am index f3212b4a..0073ff7b 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 \ @@ -335,4 +336,4 @@ tests_valgrind_potential_memleaks_SOURCES= tests/valgrind/potential-memleaks.c # we can't only build "scrypt tests/verify-strings/test_scrypt" because that # won't build the BUILT_SOURCES. test: all - $(top_srcdir)/tests/test_scrypt.sh . + $(top_srcdir)/tests/test_scrypt.sh . \ No newline at end of file diff --git a/main.c b/main.c index 2b05e252..b883d261 100644 --- a/main.c +++ b/main.c @@ -25,6 +25,9 @@ */ #include "platform.h" +#include +#include + #include #include #include @@ -96,6 +99,37 @@ scrypt_mode_info(const char * infilename) return (-1); } +/** + * same_file(infile, outfilename): + * Return non-zero if the already-open ${infile} and the path ${outfilename} + * refer to the same regular file. 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 (stat(outfilename, &sb_out)) + return (0); + + /* + * Only a regular file can be destroyed this way; opening a character + * device such as /dev/null for writing does not discard the data we + * are about to read. + */ + if (!S_ISREG(sb_in.st_mode) || !S_ISREG(sb_out.st_mode)) + return (0); + + /* The same device and inode is the same file. */ + return ((sb_in.st_dev == sb_out.st_dev) && + (sb_in.st_ino == sb_out.st_ino)); +} + /** * scrypt_mode_enc_dec(params, passphrase_entry, passphrase_arg, dec, verbose, * force_resources, infilename, outfilename): @@ -129,6 +163,17 @@ scrypt_mode_enc_dec(struct scryptenc_params params, infile = stdin; } + /* + * Refuse to write the output into the file we are reading. Opening + * it for writing truncates it, so we would destroy the data we were + * asked to encrypt or decrypt and then "succeed" in writing an + * encryption of nothing. + */ + if ((outfilename != NULL) && same_file(infile, outfilename)) { + warn0("Input and output files are the same: %s", outfilename); + goto err1; + } + /* Get the password. */ if (passphrase_entry_readpass(&passwd, passphrase_entry, passphrase_arg, "Please enter passphrase", @@ -397,4 +442,4 @@ main(int argc, char * argv[]) err0: /* Failure! */ exit(1); -} +} \ No newline at end of file diff --git a/scrypt.1 b/scrypt.1 index b81bb365..85de433c 100644 --- a/scrypt.1 +++ b/scrypt.1 @@ -63,6 +63,16 @@ 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. +.Pp .Nm Cm info provides information about the encryption parameters used for .Ar infile . @@ -248,4 +258,4 @@ The key derivation function was invented in March 2009 by Colin Percival in order to allow key files from the .Nm Tarsnap -backup system to be passphrase protected. +backup system to be passphrase protected. \ No newline at end of file diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh new file mode 100644 index 00000000..a42e8e3f --- /dev/null +++ b/tests/12-same-file.sh @@ -0,0 +1,63 @@ +#!/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}" +} \ No newline at end of file From 30ac68db8070705e07e5cd376fa155a9443be8ba Mon Sep 17 00:00:00 2001 From: tokenjunkielabs Date: Sun, 6 Sep 2026 12:20:34 +0000 Subject: [PATCH 2/8] Restore the trailing newline on the files this branch touches The previous commit was uploaded through the GitHub API by a helper which dropped the final newline of every text file it sent. Rewrite those files with their trailing newline intact; no other change. --- Makefile.am | 2 +- main.c | 2 +- scrypt.1 | 2 +- tests/12-same-file.sh | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/Makefile.am b/Makefile.am index 0073ff7b..88c64b30 100644 --- a/Makefile.am +++ b/Makefile.am @@ -336,4 +336,4 @@ tests_valgrind_potential_memleaks_SOURCES= tests/valgrind/potential-memleaks.c # we can't only build "scrypt tests/verify-strings/test_scrypt" because that # won't build the BUILT_SOURCES. test: all - $(top_srcdir)/tests/test_scrypt.sh . \ No newline at end of file + $(top_srcdir)/tests/test_scrypt.sh . diff --git a/main.c b/main.c index b883d261..150d7fca 100644 --- a/main.c +++ b/main.c @@ -442,4 +442,4 @@ main(int argc, char * argv[]) err0: /* Failure! */ exit(1); -} \ No newline at end of file +} diff --git a/scrypt.1 b/scrypt.1 index 85de433c..8b22ed4a 100644 --- a/scrypt.1 +++ b/scrypt.1 @@ -258,4 +258,4 @@ The key derivation function was invented in March 2009 by Colin Percival in order to allow key files from the .Nm Tarsnap -backup system to be passphrase protected. \ No newline at end of file +backup system to be passphrase protected. diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh index a42e8e3f..fded421c 100644 --- a/tests/12-same-file.sh +++ b/tests/12-same-file.sh @@ -60,4 +60,4 @@ scenario_cmd() { enc --passphrase dev:stdin-once \ "${plaintext_target}" "${normal_output}" echo $? > "${c_exitfile}" -} \ No newline at end of file +} From 0ccde263804d7587163bfe3f2bde97ae37be80ce Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Tue, 8 Sep 2026 09:23:49 -0400 Subject: [PATCH 3/8] Extend same-file input preservation coverage (#4) Add the authored static-alias and independent-file regression coverage to the existing test scenario. Preserve the application implementation and existing upstream submission. --- tests/12-same-file.sh | 90 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 90 insertions(+) diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh index fded421c..d23a5658 100644 --- a/tests/12-same-file.sh +++ b/tests/12-same-file.sh @@ -60,4 +60,94 @@ scenario_cmd() { 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 + + # 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}" } From dbbe855d3337db4f658ac487bc68ed3af91eda21 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sat, 19 Sep 2026 22:04:20 -0400 Subject: [PATCH 4/8] main: bind same-file guard to opened output --- main.c | 69 ++++++++++++++++++++++++++++++++++++++++++++++++++++------ 1 file changed, 62 insertions(+), 7 deletions(-) diff --git a/main.c b/main.c index 150d7fca..7115c890 100644 --- a/main.c +++ b/main.c @@ -29,11 +29,13 @@ #include #include +#include #include #include #include #include #include +#include #include "getopt.h" #include "humansize.h" @@ -130,6 +132,55 @@ same_file(FILE * infile, const char * outfilename) (sb_in.st_ino == sb_out.st_ino)); } +/** + * 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 regular file, -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 (S_ISREG(sb_in.st_mode) && S_ISREG(sb_out.st_mode) && + (sb_in.st_dev == sb_out.st_dev) && + (sb_in.st_ino == sb_out.st_ino)) { + (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): @@ -151,6 +202,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. */ @@ -164,10 +216,9 @@ scrypt_mode_enc_dec(struct scryptenc_params params, } /* - * Refuse to write the output into the file we are reading. Opening - * it for writing truncates it, so we would destroy the data we were - * asked to encrypt or decrypt and then "succeed" in writing an - * encryption of nothing. + * 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 ((outfilename != NULL) && same_file(infile, outfilename)) { warn0("Input and output files are the same: %s", outfilename); @@ -199,10 +250,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; } } From 3afd9f5c64b11a9a05185255f6cf6d1c08fa3144 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 20 Sep 2026 13:39:47 -0400 Subject: [PATCH 5/8] main: protect block-device aliases from self-overwrite --- main.c | 44 +++++++++++++++++++++++++++----------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/main.c b/main.c index 7115c890..6e5c6524 100644 --- a/main.c +++ b/main.c @@ -101,11 +101,32 @@ 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} - * refer to the same regular file. If we cannot tell -- most importantly if - * ${outfilename} does not exist yet -- return zero. + * identify the same destructive storage object. If we cannot tell -- most + * importantly if ${outfilename} does not exist yet -- return zero. */ static int same_file(FILE * infile, const char * outfilename) @@ -119,24 +140,15 @@ same_file(FILE * infile, const char * outfilename) if (stat(outfilename, &sb_out)) return (0); - /* - * Only a regular file can be destroyed this way; opening a character - * device such as /dev/null for writing does not discard the data we - * are about to read. - */ - if (!S_ISREG(sb_in.st_mode) || !S_ISREG(sb_out.st_mode)) - return (0); - - /* The same device and inode is the same file. */ - return ((sb_in.st_dev == sb_out.st_dev) && - (sb_in.st_ino == sb_out.st_ino)); + 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 regular file, -1 on error, and 0 + * 1 if both descriptors identify the same destructive object, -1 on error, + * and 0 * on success with ${outfile} set. */ static int @@ -156,9 +168,7 @@ open_output(FILE * infile, const char * outfilename, FILE ** outfile) goto err0; if (fstat(fd, &sb_out)) goto err0; - if (S_ISREG(sb_in.st_mode) && S_ISREG(sb_out.st_mode) && - (sb_in.st_dev == sb_out.st_dev) && - (sb_in.st_ino == sb_out.st_ino)) { + if (same_destructive_object(&sb_in, &sb_out)) { (void)close(fd); return (1); } From 14db0e0600340f8254fe602fd07e25a8fe3f59ad Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 20 Sep 2026 16:27:42 -0400 Subject: [PATCH 6/8] test: cover same-file output path swap --- tests/12-same-file.sh | 69 +++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 69 insertions(+) diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh index d23a5658..7706e4f6 100644 --- a/tests/12-same-file.sh +++ b/tests/12-same-file.sh @@ -119,6 +119,75 @@ scenario_cmd() { 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 \ From d38a577b1e6bfc8d80d444cc3e7ab4dcc0785e44 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 08:43:14 -0400 Subject: [PATCH 7/8] fix(main): reject stdout aliases of the input file The existing same-file guard only runs for a named output path. Append-redirected standard output can therefore write back into the input, for both enc and dec and with a named input or stdin. Compare the open stdout descriptor when no output path is supplied. Reuse the existing regular/block-device identity policy, retain named-output race protection, and keep character devices such as /dev/null valid. Extend the maintained same-file scenario with four bounded stdout cases and document the limit that a shell may already have truncated a file before scrypt starts. Actual CLI before/after: https://github.com/woahwhattheheck/scrypt/actions/runs/37202941771/job/111438168445 Standard Ubuntu 24.04.5, GCC 13.3.0, -std=c99 -O2 -Wall -Wextra; compile parent and candidate. Parent 14db0e0 exits 0 and changes the input in all four cases: enc 768 -> 1664 bytes, dec 896 -> 1664 bytes. Candidate exits 1 with the same-file diagnostic and preserves every byte in all four. Independent stdout encryption/decryption roundtrip, named-output alias, named /dev/null, and stdin/stdout /dev/null controls pass. Test script shell syntax passes. No complete suite or platform matrix run. Executed source 342abef0ff6d5f8880aed0fd3ca5df29f9362d96 and test 72d49e38a7257cb78ecd60140ce40906261cad25 are identical to this commit. The isolated validation workflow/driver remain on validation/scrypt-stdout-alias-20261004 at 120326d026ab2501f7a6072845e47a7b95b1afcc and are not included here. --- main.c | 14 ++++++++++---- scrypt.1 | 5 +++++ tests/12-same-file.sh | 41 +++++++++++++++++++++++++++++++++++++++++ 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/main.c b/main.c index 6e5c6524..342abef0 100644 --- a/main.c +++ b/main.c @@ -125,7 +125,8 @@ same_destructive_object(const struct stat * sb_in, const struct stat * sb_out) /** * same_file(infile, outfilename): * Return non-zero if the already-open ${infile} and the path ${outfilename} - * identify the same destructive storage object. If we cannot tell -- most + * 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 @@ -137,8 +138,12 @@ same_file(FILE * infile, const char * outfilename) /* If we can't stat either file, assume that they're different. */ if (fstat(fileno(infile), &sb_in)) return (0); - if (stat(outfilename, &sb_out)) + 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)); } @@ -230,8 +235,9 @@ scrypt_mode_enc_dec(struct scryptenc_params params, * repeats this check on the descriptor it actually opens, closing the * path race before any regular output file is truncated. */ - if ((outfilename != NULL) && same_file(infile, outfilename)) { - warn0("Input and output files are the same: %s", outfilename); + if (same_file(infile, outfilename)) { + warn0("Input and output files are the same: %s", + outfilename != NULL ? outfilename : "standard output"); goto err1; } diff --git a/scrypt.1 b/scrypt.1 index 8b22ed4a..ddd8686b 100644 --- a/scrypt.1 +++ b/scrypt.1 @@ -72,6 +72,11 @@ truncates 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 diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh index 7706e4f6..72d49e38 100644 --- a/tests/12-same-file.sh +++ b/tests/12-same-file.sh @@ -120,6 +120,47 @@ scenario_cmd() { 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_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 From ff39f6231fb12fc7171492d2b063b960b4bb2664 Mon Sep 17 00:00:00 2001 From: woahwhattheheck Date: Sun, 4 Oct 2026 10:31:38 -0400 Subject: [PATCH 8/8] test: isolate stdout alias fixtures under noclobber --- tests/12-same-file.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/12-same-file.sh b/tests/12-same-file.sh index 72d49e38..d7edf1e9 100644 --- a/tests/12-same-file.sh +++ b/tests/12-same-file.sh @@ -129,7 +129,7 @@ scenario_cmd() { stdout_reference="${reference_file}" fi for stdout_source in file stdin; do - stdout_target="${s_basename}-${stdout_mode}-${stdout_source}" + stdout_target="${s_basename}-stdout-${stdout_mode}-${stdout_source}" stdout_stderr="${stdout_target}.stderr" cp "${stdout_reference}" "${stdout_target}" stdout_input="${stdout_target}"