From 18285afa7689c1aecdada121ab70259ecc3054d9 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Sun, 24 Aug 2025 20:28:43 +0200 Subject: [PATCH] treewide: force log format strings to be literals luckily none of these a format strings vulnerabilities because boost::format is smart enough to throw an exception when given fewer format string arguments than are requested by specifiers Change-Id: I5fa78f0d1396263271f6e1dbcee9c0b2e9e18c34 --- lix/legacy/build-remote.cc | 2 +- lix/libcmd/installables.cc | 2 +- lix/libcmd/repl.cc | 8 ++--- lix/libmain/common-args.cc | 2 +- lix/libmain/shared.cc | 3 +- lix/libstore/build/local-derivation-goal.cc | 10 +++--- lix/libstore/build/substitution-goal.cc | 4 +-- lix/libstore/builtins/fetchurl.cc | 2 +- lix/libstore/daemon.cc | 2 +- lix/libstore/gc.cc | 2 +- lix/libstore/local-store.cc | 2 +- lix/libstore/pathlocks.cc | 4 ++- lix/libstore/remote-store.cc | 2 +- lix/libutil/current-process.cc | 21 ++++++----- lix/libutil/logging.cc | 4 +-- lix/libutil/logging.hh | 35 ++++++++++--------- lix/libutil/namespaces.cc | 2 +- lix/libutil/tarfile.cc | 2 +- lix/nix/doctor.cc | 8 ++--- lix/nix/flake.cc | 8 +++-- .../nix-eval-jobs/src/nix-eval-jobs.cc | 2 +- 21 files changed, 68 insertions(+), 59 deletions(-) diff --git a/lix/legacy/build-remote.cc b/lix/legacy/build-remote.cc index 6c6bf262f..38789c3f7 100644 --- a/lix/legacy/build-remote.cc +++ b/lix/legacy/build-remote.cc @@ -171,7 +171,7 @@ static void printSelectionFailureMessage( Uncolored(machinesFormatted) ); - printMsg(level, error.str()); + printMsg(level, "%1%", error.str()); } namespace { diff --git a/lix/libcmd/installables.cc b/lix/libcmd/installables.cc index a9fdf7748..0bc8c60cd 100644 --- a/lix/libcmd/installables.cc +++ b/lix/libcmd/installables.cc @@ -344,7 +344,7 @@ void completeFlakeRefWithFragment( } } } catch (Error & e) { - printTaggedWarning(e.msg()); + printTaggedWarning("%1%", Uncolored(e.msg())); } } diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index c2886382f..502ccc5ec 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -337,14 +337,14 @@ ReplExitStatus NixRepl::mainLoop() // input without clearing the input so far. continue; } else { - printMsg(lvlError, e.msg()); + printMsg(lvlError, "%1%", e.msg()); } } catch (EvalError & e) { - printMsg(lvlError, e.msg()); + printMsg(lvlError, "%1%", e.msg()); } catch (Error & e) { - printMsg(lvlError, e.msg()); + printMsg(lvlError, "%1%", e.msg()); } catch (Interrupted & e) { - printMsg(lvlError, e.msg()); + printMsg(lvlError, "%1%", e.msg()); } // We handled the current input fully, so we should clear it diff --git a/lix/libmain/common-args.cc b/lix/libmain/common-args.cc index a24a87f82..ba2d3ee0c 100644 --- a/lix/libmain/common-args.cc +++ b/lix/libmain/common-args.cc @@ -42,7 +42,7 @@ MixCommonArgs::MixCommonArgs(const std::string & programName) globalConfig.set(name, value); } catch (UsageError & e) { if (!getRoot().completions) { - printTaggedWarning(e.what()); + printTaggedWarning("%1%", Uncolored(e.what())); } } }}, diff --git a/lix/libmain/shared.cc b/lix/libmain/shared.cc index 6afd219d0..b1ba337d3 100644 --- a/lix/libmain/shared.cc +++ b/lix/libmain/shared.cc @@ -322,7 +322,6 @@ int handleExceptions(const std::string & programName, std::function fun) * */ bool onlyForSubcommands = baseNameOf(programName) == "lix"; - std::string error = ANSI_RED "error:" ANSI_NORMAL " "; try { fun(); } catch (Exit & e) { @@ -338,7 +337,7 @@ int handleExceptions(const std::string & programName, std::function fun) logError(e.info()); return e.info().status; } catch (const std::bad_alloc & e) { - printError(error + "out of memory"); + printError(ANSI_RED "error:" ANSI_NORMAL " out of memory"); return 1; } // Deliberately do not catch random std exceptions! We have a nice diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc index b92fac2ba..89cff969a 100644 --- a/lix/libstore/build/local-derivation-goal.cc +++ b/lix/libstore/build/local-derivation-goal.cc @@ -97,8 +97,9 @@ void handleDiffHook( diffHook, statusToString(diffRes.first)); - if (diffRes.second != "") - printError(chomp(diffRes.second)); + if (diffRes.second != "") { + printError("%1%", chomp(diffRes.second)); + } } catch (Error & error) { ErrorInfo ei = error.info(); // FIXME: wrap errors. @@ -844,7 +845,7 @@ try { ex.addTrace({}, "while setting up the build environment"); throw ex; } - debug("sandbox setup: " + msg); + debug("sandbox setup: %1%", msg); msgs.push_back(std::move(msg)); } @@ -1569,8 +1570,7 @@ void LocalDerivationGoal::runChild() #include "sandbox-minimal.sb" ; - debug("Generated sandbox profile:"); - debug(sandboxProfile); + debug("Generated sandbox profile: %1%", sandboxProfile); bool allowLocalNetworking = parsedDrv->getBoolAttr("__darwinAllowLocalNetworking"); diff --git a/lix/libstore/build/substitution-goal.cc b/lix/libstore/build/substitution-goal.cc index 7b5e61ff0..8ff25c2d0 100644 --- a/lix/libstore/build/substitution-goal.cc +++ b/lix/libstore/build/substitution-goal.cc @@ -41,7 +41,7 @@ Goal::WorkResult PathSubstitutionGoal::done( { BuildResult buildResult{.status = status}; if (errorMsg) { - debug(*errorMsg); + debug("%1%", *errorMsg); buildResult.errorMsg = *errorMsg; } return WorkResult{result, std::move(buildResult)}; @@ -263,7 +263,7 @@ try { thr.get(); break; } catch (std::exception & e) { // NOLINT(lix-foreign-exceptions) - printError(e.what()); + printError("%1%", e.what()); /* Cause the parent build to fail unless --fallback is given, or the substitute has disappeared. The latter case behaves diff --git a/lix/libstore/builtins/fetchurl.cc b/lix/libstore/builtins/fetchurl.cc index 07d258c03..029f5b636 100644 --- a/lix/libstore/builtins/fetchurl.cc +++ b/lix/libstore/builtins/fetchurl.cc @@ -75,7 +75,7 @@ void builtinFetchurl(const BasicDerivation & drv, const std::string & netrcData, ); return; } catch (Error & e) { - debug(e.what()); + debug("%1%", e.what()); } } } diff --git a/lix/libstore/daemon.cc b/lix/libstore/daemon.cc index 422af163b..f1a6eab97 100644 --- a/lix/libstore/daemon.cc +++ b/lix/libstore/daemon.cc @@ -243,7 +243,7 @@ struct ClientSettings name ); } catch (UsageError & e) { - printTaggedWarning(e.what()); + printTaggedWarning("%1%", Uncolored(e.what())); } } } diff --git a/lix/libstore/gc.cc b/lix/libstore/gc.cc index d46de9d84..2ce815dba 100644 --- a/lix/libstore/gc.cc +++ b/lix/libstore/gc.cc @@ -830,7 +830,7 @@ try { if (!kept.empty()) { printTalkative("Paths not deleted because they are still referenced by GC roots:"); for (auto &path: kept) { - printTalkative(path); + printTalkative("%1%", path); } } diff --git a/lix/libstore/local-store.cc b/lix/libstore/local-store.cc index 057407ea4..1e786f46b 100644 --- a/lix/libstore/local-store.cc +++ b/lix/libstore/local-store.cc @@ -1576,7 +1576,7 @@ try { if (TRY_AWAIT(isValidPath(i))) logError(caught->info()); else - printTaggedWarning(caught->msg()); + printTaggedWarning("%1%", Uncolored(caught->msg())); errors = true; } } diff --git a/lix/libstore/pathlocks.cc b/lix/libstore/pathlocks.cc index b56dd929a..bb790ecba 100644 --- a/lix/libstore/pathlocks.cc +++ b/lix/libstore/pathlocks.cc @@ -145,7 +145,9 @@ PathLock::lockImpl(const Path & path, std::string_view waitMsg, bool wait, Never /* Acquire an exclusive lock. */ if (!tryLockFile(fd.get(), ltWrite)) { if (wait) { - if (waitMsg != "") printError(std::string(waitMsg)); + if (waitMsg != "") { + printError("%1%", waitMsg); + } lockFile(fd.get(), ltWrite); } else { return std::nullopt; diff --git a/lix/libstore/remote-store.cc b/lix/libstore/remote-store.cc index 8362df1c3..f4b18c729 100644 --- a/lix/libstore/remote-store.cc +++ b/lix/libstore/remote-store.cc @@ -792,7 +792,7 @@ try { } else if (msg == STDERR_NEXT) - printError(chomp(TRY_AWAIT(readString(from)))); + printError("%1%", chomp(TRY_AWAIT(readString(from)))); else if (msg == STDERR_START_ACTIVITY) { auto act = TRY_AWAIT(readNum(from)); diff --git a/lix/libutil/current-process.cc b/lix/libutil/current-process.cc index b701f237f..1fab32650 100644 --- a/lix/libutil/current-process.cc +++ b/lix/libutil/current-process.cc @@ -61,15 +61,18 @@ void setStackSize(rlim_t stackSize) savedStackSize = limit.rlim_cur; limit.rlim_cur = std::min(stackSize, limit.rlim_max); if (setrlimit(RLIMIT_STACK, &limit) != 0) { - printError(HintFmt( - "Failed to increase stack size from %1% to %2% (maximum allowed " - "stack size: %3%): %4%", - savedStackSize, - stackSize, - limit.rlim_max, - std::strerror(errno) - ) - .str()); + printError( + "%1%", + HintFmt( + "Failed to increase stack size from %1% to %2% (maximum allowed " + "stack size: %3%): %4%", + savedStackSize, + stackSize, + limit.rlim_max, + std::strerror(errno) + ) + .str() + ); } } } diff --git a/lix/libutil/logging.cc b/lix/libutil/logging.cc index e2f3fcd69..2ce7e1903 100644 --- a/lix/libutil/logging.cc +++ b/lix/libutil/logging.cc @@ -95,11 +95,11 @@ public: { if (type == resBuildLogLine && printBuildLogs) { auto lastLine = fields[0].s; - printError(lastLine); + printError("%1%", lastLine); } else if (type == resPostBuildLogLine) { auto lastLine = fields[0].s; - printError("post-build-hook: " + lastLine); + printError("post-build-hook: %1%", lastLine); } } }; diff --git a/lix/libutil/logging.hh b/lix/libutil/logging.hh index c157a6d2c..28ecd7412 100644 --- a/lix/libutil/logging.hh +++ b/lix/libutil/logging.hh @@ -255,27 +255,28 @@ extern Verbosity verbosity; /** * Print a string message if the current log level is at least the specified * level. Note that this has to be implemented as a macro to ensure that the - * arguments are evaluated lazily. + * arguments are evaluated lazily. The format string *must* be a literal. */ -#define printMsgUsing(loggerParam, level, args...) \ - do { \ - auto _lix_logger_print_lvl = level; \ - if (_lix_logger_print_lvl <= ::nix::verbosity) { \ - loggerParam->log(_lix_logger_print_lvl, ::nix::fmt(args)); \ - } \ +#define printMsgUsing(loggerParam, level, fs, args...) \ + do { \ + auto _lix_logger_print_lvl = level; \ + const char * _lix_format = [](const char(&_lix_fs)[N]) { return _lix_fs; }(fs); \ + if (_lix_logger_print_lvl <= ::nix::verbosity) { \ + loggerParam->log(_lix_logger_print_lvl, ::nix::fmt(_lix_format, ##args)); \ + } \ } while (0) -#define printMsg(level, args...) printMsgUsing(::nix::logger, level, args) +#define printMsg(level, fs, args...) printMsgUsing(::nix::logger, level, fs, ##args) -#define printWarning(args...) printMsg(::nix::lvlWarn, args) -#define printError(args...) printMsg(::nix::lvlError, args) -#define notice(args...) printMsg(::nix::lvlNotice, args) -#define printInfo(args...) printMsg(::nix::lvlInfo, args) -#define printTalkative(args...) printMsg(::nix::lvlTalkative, args) -#define debug(args...) printMsg(::nix::lvlDebug, args) -#define vomit(args...) printMsg(::nix::lvlVomit, args) +#define printWarning(fs, args...) printMsg(::nix::lvlWarn, fs, ##args) +#define printError(fs, args...) printMsg(::nix::lvlError, fs, ##args) +#define notice(fs, args...) printMsg(::nix::lvlNotice, fs, ##args) +#define printInfo(fs, args...) printMsg(::nix::lvlInfo, fs, ##args) +#define printTalkative(fs, args...) printMsg(::nix::lvlTalkative, fs, ##args) +#define debug(fs, args...) printMsg(::nix::lvlDebug, fs, ##args) +#define vomit(fs, args...) printMsg(::nix::lvlVomit, fs, ##args) -#define printTaggedWarning(args...) \ - printWarning(ANSI_WARNING "warning:" ANSI_NORMAL " %1%", ::nix::HintFmt(args).str()) +#define printTaggedWarning(fs, args...) \ + printWarning("%1%", ::nix::HintFmt(ANSI_WARNING "warning:" ANSI_NORMAL " " fs, ##args).str()) void writeLogsToStderr(std::string_view s); diff --git a/lix/libutil/namespaces.cc b/lix/libutil/namespaces.cc index fbd285000..f2b61b08c 100644 --- a/lix/libutil/namespaces.cc +++ b/lix/libutil/namespaces.cc @@ -52,7 +52,7 @@ void restoreMountNamespace() if (chdir(savedCwd.c_str()) == -1) throw SysError("restoring cwd"); } catch (Error & e) { - debug(e.msg()); + debug("%1%", e.msg()); } #endif } diff --git a/lix/libutil/tarfile.cc b/lix/libutil/tarfile.cc index c83e5dd74..ff34c3376 100644 --- a/lix/libutil/tarfile.cc +++ b/lix/libutil/tarfile.cc @@ -94,7 +94,7 @@ static void extract_archive(TarArchive & archive, const Path & destDir) if (!name) throw Error("cannot get archive member name: %s", archive_error_string(archive.archive)); if (r == ARCHIVE_WARN) - printTaggedWarning(archive_error_string(archive.archive)); + printTaggedWarning("%1%", Uncolored(archive_error_string(archive.archive))); else archive.check(r); diff --git a/lix/nix/doctor.cc b/lix/nix/doctor.cc index 81faa6c61..0a244045c 100644 --- a/lix/nix/doctor.cc +++ b/lix/nix/doctor.cc @@ -25,17 +25,17 @@ std::string formatProtocol(unsigned int proto) } bool checkPass(const std::string & msg) { - notice(ANSI_GREEN "[PASS] " ANSI_NORMAL + msg); + notice(ANSI_GREEN "[PASS]" ANSI_NORMAL " %1%", msg); return true; } bool checkFail(const std::string & msg) { - notice(ANSI_RED "[FAIL] " ANSI_NORMAL + msg); + notice(ANSI_RED "[FAIL]" ANSI_NORMAL " %1%", msg); return false; } void checkInfo(const std::string & msg) { - notice(ANSI_BLUE "[INFO] " ANSI_NORMAL + msg); + notice(ANSI_BLUE "[INFO]" ANSI_NORMAL " %1%", msg); } } @@ -61,7 +61,7 @@ struct CmdDoctor : StoreCommand void run(ref store) override { - printInfo("Running checks against store uri: " + store->getUri()); + printInfo("Running checks against store uri %1%", store->getUri()); if (store.try_cast_shared()) { success &= checkNixInPath(); diff --git a/lix/nix/flake.cc b/lix/nix/flake.cc index 268975860..22c0a3049 100644 --- a/lix/nix/flake.cc +++ b/lix/nix/flake.cc @@ -954,8 +954,12 @@ struct CmdFlakeInitCommon : virtual Args, EvalCommand } auto welcomeText = cursor->maybeGetAttr(*evalState, "welcomeText"); if (welcomeText) { - notice("\n"); - notice(renderMarkdownToTerminal(welcomeText->getString(*evalState), StandardOutputStream::Stderr)); + notice( + "\n%1%", + renderMarkdownToTerminal( + welcomeText->getString(*evalState), StandardOutputStream::Stderr + ) + ); } if (!conflictedFiles.empty()) diff --git a/subprojects/nix-eval-jobs/src/nix-eval-jobs.cc b/subprojects/nix-eval-jobs/src/nix-eval-jobs.cc index da85cb198..0602a9067 100644 --- a/subprojects/nix-eval-jobs/src/nix-eval-jobs.cc +++ b/subprojects/nix-eval-jobs/src/nix-eval-jobs.cc @@ -77,7 +77,7 @@ struct Proc { JSON err; auto msg = e.msg(); err["error"] = nix::filterANSIEscapes(msg, true); - printError(msg); + printError("%1%", msg); if (tryWriteLine(to->get(), err.dump()) < 0) { return; // main process died };