diff --git a/src/htsbauth.c b/src/htsbauth.c index caac90c55..2b96827d2 100644 --- a/src/htsbauth.c +++ b/src/htsbauth.c @@ -41,6 +41,7 @@ Please visit our Website: http://www.httrack.com /* specific definitions */ #include "htsglobal.h" #include "htslib.h" +#include "htslines.h" #include "htscore.h" #include "htspsl.h" #include "htsdate.h" @@ -1099,25 +1100,6 @@ int cookie_load_from(httrackp *opt, t_cookie *cookie, const char *fpath, return -1; } -/* Open name for writing, creating it if needed, and return NULL on failure. - The file is owner-only on Unix because it holds session cookies. */ -static FILE *cookie_create(const char *name) { - char catbuff[CATBUFF_SIZE]; -#ifdef _WIN32 - return FOPEN(fconv(catbuff, sizeof(catbuff), name), "wb"); -#else - const int fd = open(fconv(catbuff, sizeof(catbuff), name), - O_WRONLY | O_CREAT | O_TRUNC, HTS_PROTECT_FILE); - FILE *fp = fd != -1 ? fdopen(fd, "wb") : NULL; - - if (fd != -1 && fp == NULL) - close(fd); /* fdopen failed: don't leak the descriptor */ - if (fp != NULL) /* O_CREAT's mode skips pre-existing jars: tighten those */ - (void) fchmod(fd, HTS_PROTECT_FILE); - return fp; -#endif -} - /* Flush fp to the disk and close it. Did every step succeed? */ static hts_boolean cookie_close_synced(FILE *fp) { // a crash after the rename must not leave an empty file behind it @@ -1139,7 +1121,7 @@ int cookie_save_fault_selftest = 0; success. */ static int cookie_write(t_cookie *cookie, const char *name) { char BIGSTK line[8192]; - FILE *fp = cookie_create(name); + FILE *fp = hts_fopen_private(name, "wb"); if (fp) { const char *a = cookie->data; @@ -1189,7 +1171,7 @@ static hts_boolean cookie_copy(const char *src, const char *dst) { const int fault = cookie_save_fault_selftest; char buf[8192]; FILE *const in = FOPEN(src, "rb"); - FILE *const out = in != NULL ? cookie_create(dst) : NULL; + FILE *const out = in != NULL ? hts_fopen_private(dst, "wb") : NULL; hts_boolean ok = out != NULL; size_t n; diff --git a/src/htscache.c b/src/htscache.c index a71f76c48..51f679af9 100644 --- a/src/htscache.c +++ b/src/htscache.c @@ -41,6 +41,7 @@ Please visit our Website: http://www.httrack.com #include "htsio.h" #include "htsbasenet.h" #include "htsmd5.h" +#include "htslines.h" #include #include @@ -1470,7 +1471,14 @@ void cache_init(cache_back * cache, httrackp * opt) { StringBuff(opt->path_log), "hts-cache/")); { - /* Create ZIP file cache */ + /* owner-only before an entry names a URL: zip's "wb" keeps the mode */ + FILE *const zfp = hts_fopen_private( + fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-cache/new.zip"), + "wb"); + + if (zfp != NULL) + fclose(zfp); cache->zipOutput = (void *) hts_zipOpen_utf8( fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), StringBuff(opt->path_log), "hts-cache/new.zip"), @@ -1514,10 +1522,11 @@ void cache_init(cache_back * cache, httrackp * opt) { fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), StringBuff(opt->path_log), "hts-cache/old.txt")); // ouvrir - cache->txt = - FOPEN(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), - StringBuff(opt->path_log), "hts-cache/new.txt"), - "wb"); + /* owner-only: it lists URLs with their passwords */ + cache->txt = hts_fopen_private( + fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-cache/new.txt"), + "wb"); if (cache->txt) { fprintf(cache->txt, "date\tsize'/'remotesize\tflags(request:Update,Range state:File response:Modified,Chunked,gZipped)\t"); diff --git a/src/htscoremain.c b/src/htscoremain.c index 5f37eb582..a1b4275fb 100644 --- a/src/htscoremain.c +++ b/src/htscoremain.c @@ -346,7 +346,7 @@ HTSEXT_API int hts_main(int argc, char **argv) { written at all. */ static hts_boolean write_progress_lock(httrackp *opt, const char *path, const char *t, int argc, char **argv) { - FILE *fp = FOPEN(path, "wb"); + FILE *fp = hts_fopen_private(path, "wb"); /* it echoes argv */ int i; if (fp == NULL) @@ -2946,13 +2946,16 @@ static int hts_main_internal(int argc, char **argv, httrackp * opt) { /* Check FS directory structure created */ structcheck_utf8(StringBuff(opt->path_log)); - opt->log = FOPEN(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), - StringBuff(opt->path_log), "hts-log.txt"), - "w"); - if (httrack_logmode == 2) - opt->errlog = FOPEN(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), - StringBuff(opt->path_log), "hts-err.txt"), + /* owner-only: the log echoes the command line and URLs with passwords */ + opt->log = + hts_fopen_private(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-log.txt"), "w"); + if (httrack_logmode == 2) + opt->errlog = + hts_fopen_private(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-err.txt"), + "w"); else opt->errlog = opt->log; if (opt->log == NULL) { @@ -2984,9 +2987,11 @@ static int hts_main_internal(int argc, char **argv, httrackp * opt) { // -%H may run before path_log exists, so create it first. if (_DEBUG_HEAD) { structcheck_utf8(StringBuff(opt->path_log)); - ioinfo = FOPEN(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), - StringBuff(opt->path_log), "hts-ioinfo.txt"), - "wb"); + /* owner-only: it records request headers, Authorization included */ + ioinfo = hts_fopen_private( + fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-ioinfo.txt"), + "wb"); if (ioinfo == NULL) hts_log_print(opt, LOG_WARNING | LOG_ERRNO, "unable to create header debug file %s", @@ -3063,9 +3068,11 @@ static int hts_main_internal(int argc, char **argv, httrackp * opt) { (OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), StringBuff(opt->path_log), "hts-cache"), HTS_PROTECT_FOLDER); #endif - fp = FOPEN(fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), - StringBuff(opt->path_log), "hts-cache/doit.log"), - "wb"); + /* owner-only: the command line may carry credentials */ + fp = hts_fopen_private( + fconcat(OPT_GET_BUFF(opt), OPT_GET_BUFF_SIZE(opt), + StringBuff(opt->path_log), "hts-cache/doit.log"), + "wb"); if (fp) { String line = STRING_EMPTY; diff --git a/src/htslines.c b/src/htslines.c index d7a95cfd0..49c7d1521 100644 --- a/src/htslines.c +++ b/src/htslines.c @@ -27,15 +27,46 @@ Please visit our Website: http://www.httrack.com /* ------------------------------------------------------------ */ /* File: htslines.c */ -/* line readers for config files, shared by the engine */ -/* and htsserver */ +/* line readers for config files and an owner-only file */ +/* opener, shared by the engine and htsserver */ /* Author: Xavier Roche */ /* ------------------------------------------------------------ */ #include "htslines.h" #include "htslib.h" +#include #include +#ifndef _WIN32 +#include +#include +#include +#endif + +FILE *hts_fopen_private(const char *name, const char *mode) { + char catbuff[CATBUFF_SIZE]; + + /* O_TRUNC below would silently turn an append into a rewrite */ + if (strcmp(mode, "w") != 0 && strcmp(mode, "wb") != 0) { + errno = EINVAL; + return NULL; + } +#ifdef _WIN32 + return FOPEN(fconv(catbuff, sizeof(catbuff), name), mode); +#else + { + const int fd = open(fconv(catbuff, sizeof(catbuff), name), + O_WRONLY | O_CREAT | O_TRUNC, HTS_PROTECT_FILE); + FILE *fp = fd != -1 ? fdopen(fd, mode) : NULL; + + if (fd != -1 && fp == NULL) + close(fd); /* fdopen failed: don't leak the descriptor */ + if (fp != NULL) /* O_CREAT's mode skips pre-existing files: tighten those */ + (void) fchmod(fd, HTS_PROTECT_FILE); + return fp; + } +#endif +} /* Is c one of the bytes flags drop? */ static hts_boolean line_drops(int c, int flags) { diff --git a/src/htslines.h b/src/htslines.h index 0f55e440a..47f7fe244 100644 --- a/src/htslines.h +++ b/src/htslines.h @@ -27,8 +27,8 @@ Please visit our Website: http://www.httrack.com /* ------------------------------------------------------------ */ /* File: htslines.h */ -/* line readers for config files, shared by the engine */ -/* and htsserver */ +/* line readers for config files and an owner-only file */ +/* opener, shared by the engine and htsserver */ /* Author: Xavier Roche */ /* ------------------------------------------------------------ */ @@ -41,6 +41,10 @@ Please visit our Website: http://www.httrack.com #include #include +/* Open UTF-8 path name for writing ("w" or "wb" only), owner-only on Unix, + truncating it; NULL on failure. Links are followed, as fopen() does. */ +FILE *hts_fopen_private(const char *name, const char *mode); + /* Flags for the readers below, which always drop CR. */ #define HTS_LINE_DROP_TAB 1 /* TAB */ #define HTS_LINE_DROP_NUL 2 /* NUL */ diff --git a/src/htslines_selftest.c b/src/htslines_selftest.c index ad5d0ccd4..7fb0c5a7c 100644 --- a/src/htslines_selftest.c +++ b/src/htslines_selftest.c @@ -401,6 +401,78 @@ static int st_linediff(httrackp *opt, int argc, char **argv) { return k.failures == 0 ? 0 : 1; } +/* hts_fopen_private() under umask 022: fstat() right after the open, before any + write, so a file made private only once written fails too. Arg: a directory. + */ +static int st_fopen_private(httrackp *opt, int argc, char **argv) { + char path[HTS_URLMAXSIZE * 2]; + char data[8]; + int failures = 0; + FILE *fp; + size_t i; + static const char *const refused[] = {"a", "ab", "r", "r+", "w+", "wb+"}; + + (void) opt; + if (argc < 1) { + printf("usage: -#test=fopen-private \n"); + return 1; + } + snprintf(path, sizeof(path), "%s/kept", argv[0]); + fp = FOPEN(path, "wb"); + if (fp == NULL || fputs("kept", fp) == EOF || fclose(fp) != 0) { + printf("fopen-private: cannot write %s\n", path); + return 1; + } + for (i = 0; i < sizeof(refused) / sizeof(refused[0]); i++) { + errno = 0; + fp = hts_fopen_private(path, refused[i]); + if (fp != NULL || errno != EINVAL) { + printf("fopen-private: mode \"%s\" was not refused\n", refused[i]); + failures++; + if (fp != NULL) + fclose(fp); + } + } + fp = FOPEN(path, "rb"); + if (fp == NULL || fread(data, 1, sizeof(data), fp) != 4 || + memcmp(data, "kept", 4) != 0) { + printf("fopen-private: a refused mode changed the file\n"); + failures++; + } + if (fp != NULL) + fclose(fp); +#ifndef _WIN32 + { + const mode_t old = umask(022); + static const char *const names[] = {"fresh", "kept"}; + + /* "kept" exists at 0644 from older runs: it must be tightened and emptied + */ + snprintf(path, sizeof(path), "%s/kept", argv[0]); + (void) chmod(path, 0644); + for (i = 0; i < sizeof(names) / sizeof(names[0]); i++) { + struct stat st; + + snprintf(path, sizeof(path), "%s/%s", argv[0], names[i]); + fp = hts_fopen_private(path, i == 0 ? "w" : "wb"); + if (fp == NULL || fstat(fileno(fp), &st) != 0) { + printf("fopen-private: cannot open %s\n", names[i]); + failures++; + } else if ((st.st_mode & 0777) != 0600 || st.st_size != 0) { + printf("fopen-private: %s is mode %03o size %d before any write\n", + names[i], (unsigned) (st.st_mode & 0777), (int) st.st_size); + failures++; + } + if (fp != NULL) + fclose(fp); + } + (void) umask(old); + } +#endif + printf("fopen-private: %s\n", failures == 0 ? "OK" : "FAIL"); + return failures == 0 ? 0 : 1; +} + /* ------------------------------------------------------------ */ /* Registry: this module's tests, in the order -#test lists them. */ /* ------------------------------------------------------------ */ @@ -410,5 +482,8 @@ const struct selftest_entry selftests_lines[] = { "config-file line reader: [line] per read, ! when cut", st_readline}, {"linediff", "", "line readers against the legacy ones they replaced", st_linediff}, + {"fopen-private", "", + "owner-only opener: 0600 before the first write, append refused", + st_fopen_private}, {NULL, NULL, NULL, NULL}, }; diff --git a/src/htsserver.c b/src/htsserver.c index de08fd40e..b822841ef 100644 --- a/src/htsserver.c +++ b/src/htsserver.c @@ -1936,7 +1936,8 @@ int smallserver(T_SOC soc, char *url, char *method, char *data, char *path) { StringCopy(website, StringBuff(fspath)); StringCat(tmpbuff, "winprofile.ini"); - fp = fopen(StringBuff(tmpbuff), "wb"); + /* owner-only: the profile may hold proxy credentials */ + fp = hts_fopen_private(StringBuff(tmpbuff), "wb"); if (fp != NULL) { int count; diff --git a/tests/658_local-private-run-files.test b/tests/658_local-private-run-files.test new file mode 100644 index 000000000..bc008ea1f --- /dev/null +++ b/tests/658_local-private-run-files.test @@ -0,0 +1,105 @@ +#!/bin/bash +# +# The files that echo the command line, request headers or URLs, any of which +# can hold a password, are owner-only even under a permissive umask. + +set -euo pipefail + +# shellcheck source=tests/crawllib.sh +. "${0%"${0##*/}"}./crawllib.sh" +# shellcheck source=tests/webhttracklib.sh +. "${0%"${0##*/}"}./webhttracklib.sh" + +skip_on_windows "no POSIX file modes" +find_python >/dev/null || skip "python3 not found" + +tmpdir=$(mktemp -d "${TMPDIR:-/tmp}/httrack_private.XXXXXX") || exit 1 +crawlpid= +cleanup() { + if test -n "$crawlpid"; then + kill_tree "$crawlpid" + wait "$crawlpid" 2>/dev/null || true + fi + rm -rf "$tmpdir" +} +cleanup_push cleanup + +require_httrack +htsserver_require +local_server_start +umask 022 + +mode_of() { + stat -c '%a' "$1" 2>/dev/null || stat -f '%Lp' "$1" +} + +assert_private() { # assert_private WHEN DIR FILE... + local when=$1 dir=$2 f m + shift 2 + for f in "$@"; do + test -f "$dir/$f" || fail "$when: no $f" + m=$(mode_of "$dir/$f") + test "$m" = 600 || fail "$when: $f is mode $m, want 600" + done + echo "$when: mode 600 for $*" +} + +# Control: the probe must see a group- and world-readable file. +: >"$tmpdir/control" +test "$(mode_of "$tmpdir/control")" = 644 || + fail "umask 022 made mode $(mode_of "$tmpdir/control"), so 600 proves nothing" + +# The opener itself, read before the first write and so before a late chmod. +mkdir "$tmpdir/st" +httrack -#test=fopen-private "$tmpdir/st" >"$tmpdir/st.log" 2>&1 || + fail "fopen-private self-test failed: $(<"$tmpdir/st.log")" +grep -q '^fopen-private: OK$' "$tmpdir/st.log" || + fail "fopen-private self-test said: $(<"$tmpdir/st.log")" + +# -f splits the errors into hts-err.txt, and -%H writes hts-ioinfo.txt. +out="$tmpdir/mirror" +files=(hts-cache/doit.log hts-log.txt hts-err.txt hts-ioinfo.txt + hts-cache/new.txt hts-cache/new.zip) +local_crawl --log "$tmpdir/log1" -O "$out" --quiet --robots=0 --timeout=30 \ + --retries=0 -f -%H \ + "http://user:secret@127.0.0.1:$SRV_PORT/fetchmeta/index.html" || + fail "crawl failed: $(<"$tmpdir/log1")" +grep -q secret "$out/hts-cache/doit.log" || fail "doit.log does not hold the URL" +assert_private "first crawl" "$out" "${files[@]}" + +# doit.log is rewritten in place, so a 0644 one from an older run must be +# tightened. The logs are unlinked before they are made, so they cannot test it. +chmod 644 "$out/hts-cache/doit.log" +local_crawl --log "$tmpdir/log2" -O "$out" --quiet --update || + fail "update failed: $(<"$tmpdir/log2")" +assert_private "update" "$out" hts-cache/doit.log + +# A crawl held on a trickling page: the lock exists only while it runs, and a +# file made private only at the end of the run is still 644 here. +slow="$tmpdir/slow" +httrack -O "$slow" --quiet --robots=0 --timeout=120 --retries=0 -c1 \ + "$BASEURL/trickle/index.html" >"$tmpdir/log3" 2>&1 & +crawlpid=$! +for _ in $(seq 1 300); do + test -e "$slow/hts-cache/new.txt" && test -e "$slow/hts-in_progress.lock" && + break + kill -0 "$crawlpid" 2>/dev/null || + fail "the trickle crawl ended early: $(<"$tmpdir/log3")" + sleep 0.1 +done +assert_private "running crawl" "$slow" hts-in_progress.lock hts-cache/doit.log \ + hts-log.txt hts-cache/new.txt hts-cache/new.zip + +# WebHTTrack saves the project's winprofile.ini, which can carry a proxy password. +web="$tmpdir/web" +mkdir -p "$web/websites" +cleanup_push htsserver_cleanup +htsserver_start --home "$web" +sid=$(htsserver_sid) +test -n "$sid" || fail "no session id from htsserver" +body="sid=$sid&path=$(nativepath "$web/websites")&projname=proj&command=httrack" +body="$body&command_do=save&winprofile=Proxy%3Duser%3Asecret%40proxy" +htsserver_post "$body" /server/step4.html >"$tmpdir/web.log" 2>&1 || + fail "the save failed: $(<"$tmpdir/web.log")" +assert_private "webhttrack save" "$web/websites/proj" hts-cache/winprofile.ini +htsserver_cleanup diff --git a/tests/ci-windows-suite.sh b/tests/ci-windows-suite.sh index a8b676209..1b86d2ce7 100644 --- a/tests/ci-windows-suite.sh +++ b/tests/ci-windows-suite.sh @@ -325,6 +325,7 @@ ci_suite_heartbeat() { # and Windows has a disposition for none of that set. # engine-sigterm-noatexit forks a child per case and raises SIGINT at it, and # Windows has neither fork() nor that disposition. +# private-run-files checks POSIX file modes, which Windows does not have. expected_skips_msys="01_engine-usercommand.test 01_engine-footer-overflow.test 253_local-ftp-close-once.test @@ -369,7 +370,8 @@ expected_skips_msys="01_engine-usercommand.test 465_local-mirror-completed.test 481_local-chunked-oom.test 494_local-mptcp.test -01_engine-mptcp.test" +01_engine-mptcp.test +658_local-private-run-files.test" # Measured, not predicted: windows-build run 33927128153, both platforms alike. # Written out rather than derived from the msys list above: the two lists are @@ -394,6 +396,7 @@ expected_skips_msys="01_engine-usercommand.test # inert but the replaying bash runs it. # engine-sigterm-noatexit forks a child per case and raises SIGINT at it, and # Windows has neither fork() nor that disposition. +# private-run-files checks POSIX file modes, which Windows does not have. expected_skips_wsl2="01_engine-usercommand.test 01_engine-footer-overflow.test 253_local-ftp-close-once.test @@ -439,7 +442,8 @@ expected_skips_wsl2="01_engine-usercommand.test 294_local-wizard-eof.test 481_local-chunked-oom.test 494_local-mptcp.test -01_engine-mptcp.test" +01_engine-mptcp.test +658_local-private-run-files.test" # Sets ci_skip_list to the pinned skip set for backend $1, failing loudly if # there is none: an unknown backend must never fall back to an empty list,