From c948b350fb67550fade2209bbfa49b1d668285c8 Mon Sep 17 00:00:00 2001 From: eldritch horrors Date: Tue, 17 Dec 2024 16:25:00 +0100 Subject: [PATCH] libutil: add CheckedSourcePath for accessing things SourcePath only manipulates path names now. all accesses must go through a checked path going forward to ensure we don't escape restriction lists of pure and restricted evaluation. if a directory path is checked it can safely be assumed that the directory itself is allowed, and its contents will likewise be safe to access. it is tempting to assumed that contents will also be fine, but that's only true if the content is not a symlink. Change-Id: Icec3098d53fe9dce50997954ba958fe4f304d59b --- lix/legacy/nix-env.cc | 6 +- lix/libcmd/repl.cc | 4 +- lix/libexpr/eval.cc | 25 ++++---- lix/libexpr/eval.hh | 10 +-- lix/libexpr/primops.cc | 22 +++---- lix/libexpr/value-to-xml.cc | 2 +- lix/libfetchers/fetch-to-store.cc | 2 +- lix/libfetchers/fetch-to-store.hh | 2 +- lix/libutil/position.cc | 2 +- lix/libutil/position.hh | 2 +- lix/libutil/source-path.cc | 10 +-- lix/libutil/source-path.hh | 103 ++++++++++++++++++------------ 12 files changed, 107 insertions(+), 83 deletions(-) diff --git a/lix/legacy/nix-env.cc b/lix/legacy/nix-env.cc index e83522dc6..1a98bedaa 100644 --- a/lix/legacy/nix-env.cc +++ b/lix/legacy/nix-env.cc @@ -96,7 +96,7 @@ static bool parseInstallSourceOptions(Globals & globals, } -static bool isNixExpr(EvalPaths & paths, const SourcePath & path, struct InputAccessor::Stat & st) +static bool isNixExpr(EvalPaths & paths, const CheckedSourcePath & path, struct InputAccessor::Stat & st) { if (st.type == InputAccessor::tRegular) { return true; @@ -113,7 +113,7 @@ static constexpr size_t maxAttrs = 1024; static void getAllExprs(Evaluator & state, - const SourcePath & path, StringSet & seen, BindingsBuilder & attrs) + const CheckedSourcePath & path, StringSet & seen, BindingsBuilder & attrs) { StringSet namesSorted; for (auto & [name, _] : path.readDirectory()) namesSorted.insert(name); @@ -124,7 +124,7 @@ static void getAllExprs(Evaluator & state, are implemented using profiles). */ if (i == "manifest.nix") continue; - SourcePath path2 = state.paths.checkSourcePath(path + i); + auto path2 = state.paths.checkSourcePath(path + i); InputAccessor::Stat st; try { diff --git a/lix/libcmd/repl.cc b/lix/libcmd/repl.cc index 56d9da9d8..86feec53f 100644 --- a/lix/libcmd/repl.cc +++ b/lix/libcmd/repl.cc @@ -655,7 +655,7 @@ ProcessLineResult NixRepl::processLine(std::string line) return {path, 0}; } else if (v.isLambda()) { auto pos = evaluator.positions[v.lambda.fun->pos]; - if (auto path = std::get_if(&pos.origin)) + if (auto path = std::get_if(&pos.origin)) return {*path, pos.line}; else throw Error("'%s' cannot be shown in an editor", pos); @@ -820,7 +820,7 @@ ProcessLineResult NixRepl::processLine(std::string line) logger->cout(trim(renderMarkdownToTerminal(markdown))); } else if (v.isLambda()) { auto pos = evaluator.positions[v.lambda.fun->pos]; - if (auto path = std::get_if(&pos.origin)) { + if (auto path = std::get_if(&pos.origin)) { // Path and position have now been obtained, feed to nix-doc library to get data. auto docComment = lambdaDocsForPos(*path, pos); if (!docComment) { diff --git a/lix/libexpr/eval.cc b/lix/libexpr/eval.cc index b59d52459..6217bc555 100644 --- a/lix/libexpr/eval.cc +++ b/lix/libexpr/eval.cc @@ -379,9 +379,9 @@ void EvalPaths::allowAndSetStorePathString(const StorePath & storePath, Value & mkStorePathString(storePath, v); } -SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) +CheckedSourcePath EvalPaths::checkSourcePath(const SourcePath & path_) { - if (!allowedPaths) return path_; + if (!allowedPaths) return auto(path_).unsafeIntoChecked(); auto i = resolvedPaths.find(path_.canonical().abs()); if (i != resolvedPaths.end()) @@ -395,7 +395,9 @@ SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) */ Path abspath = canonPath(path_.canonical().abs()); - if (abspath.starts_with(corepkgsPrefix)) return CanonPath(abspath); + if (abspath.starts_with(corepkgsPrefix)) { + return SourcePath(CanonPath(abspath)).unsafeIntoChecked(); + } for (auto & i : *allowedPaths) { if (isDirOrInDir(abspath, i)) { @@ -417,8 +419,9 @@ SourcePath EvalPaths::checkSourcePath(const SourcePath & path_) for (auto & i : *allowedPaths) { if (isDirOrInDir(path.canonical().abs(), i)) { - resolvedPaths.insert_or_assign(path_.canonical().abs(), path); - return path; + auto checked = path.unsafeIntoChecked(); + resolvedPaths.insert_or_assign(path_.canonical().abs(), checked); + return checked; } } @@ -803,7 +806,7 @@ void Evaluator::evalLazily(Expr & e, Value & v) void EvalState::mkPos(Value & v, PosIdx p) { auto origin = ctx.positions.originOf(p); - if (auto path = std::get_if(&origin)) { + if (auto path = std::get_if(&origin)) { auto attrs = ctx.buildBindings(3); attrs.alloc(ctx.s.file).mkString(path->to_string()); makePositionThunks(*this, p, attrs.alloc(ctx.s.line), attrs.alloc(ctx.s.column)); @@ -2621,7 +2624,7 @@ void Evaluator::printStatistics() else obj["name"] = nullptr; if (auto pos = positions[fun->pos]) { - if (auto path = std::get_if(&pos.origin)) + if (auto path = std::get_if(&pos.origin)) obj["file"] = path->to_string(); obj["line"] = pos.line; obj["column"] = pos.column; @@ -2636,7 +2639,7 @@ void Evaluator::printStatistics() for (auto & i : stats.attrSelects) { json obj = json::object(); if (auto pos = positions[i.first]) { - if (auto path = std::get_if(&pos.origin)) + if (auto path = std::get_if(&pos.origin)) obj["file"] = path->to_string(); obj["line"] = pos.line; obj["column"] = pos.column; @@ -2661,7 +2664,7 @@ void Evaluator::printStatistics() } -SourcePath EvalPaths::resolveExprPath(SourcePath path_) +CheckedSourcePath EvalPaths::resolveExprPath(SourcePath path_) { auto path = checkSourcePath(path_); unsigned int followCount = 0, maxFollow = 1024; @@ -2686,13 +2689,13 @@ SourcePath EvalPaths::resolveExprPath(SourcePath path_) } -Expr & Evaluator::parseExprFromFile(const SourcePath & path) +Expr & Evaluator::parseExprFromFile(const CheckedSourcePath & path) { return parseExprFromFile(path, builtins.staticEnv); } -Expr & Evaluator::parseExprFromFile(const SourcePath & path, std::shared_ptr & staticEnv) +Expr & Evaluator::parseExprFromFile(const CheckedSourcePath & path, std::shared_ptr & staticEnv) { auto buffer = path.readFile(); return *parse(buffer.data(), buffer.size(), Pos::Origin(path), path.parent(), staticEnv); diff --git a/lix/libexpr/eval.hh b/lix/libexpr/eval.hh index bb87add5d..5c0ddc978 100644 --- a/lix/libexpr/eval.hh +++ b/lix/libexpr/eval.hh @@ -393,7 +393,7 @@ private: /** * Cache used by checkSourcePath(). */ - std::unordered_map resolvedPaths; + std::unordered_map resolvedPaths; public: /** @@ -416,12 +416,12 @@ public: * Check whether access to a path is allowed and throw an error if * not. Otherwise return the canonicalised path. */ - SourcePath checkSourcePath(const SourcePath & path); + CheckedSourcePath checkSourcePath(const SourcePath & path); /** * If `path` refers to a directory, then append "/default.nix". */ - SourcePath resolveExprPath(SourcePath path); + CheckedSourcePath resolveExprPath(SourcePath path); void checkURI(const std::string & uri); @@ -536,8 +536,8 @@ public: /** * Parse a Nix expression from the specified file. */ - Expr & parseExprFromFile(const SourcePath & path); - Expr & parseExprFromFile(const SourcePath & path, std::shared_ptr & staticEnv); + Expr & parseExprFromFile(const CheckedSourcePath & path); + Expr & parseExprFromFile(const CheckedSourcePath & path, std::shared_ptr & staticEnv); /** * Parse a Nix expression from the specified string. diff --git a/lix/libexpr/primops.cc b/lix/libexpr/primops.cc index d9ced1df7..cfbf86ced 100644 --- a/lix/libexpr/primops.cc +++ b/lix/libexpr/primops.cc @@ -117,12 +117,7 @@ StringMap EvalPaths::realiseContext(const NixStringContext & context) return res; } -struct RealisePathFlags { - // Whether to check that the path is allowed in pure eval mode - bool checkForPureEval = true; -}; - -static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, const RealisePathFlags flags = {}) +static auto realisePath(EvalState & state, const PosIdx pos, Value & v, auto checkFn) { NixStringContext context; @@ -131,17 +126,20 @@ static SourcePath realisePath(EvalState & state, const PosIdx pos, Value & v, co try { StringMap rewrites = state.ctx.paths.realiseContext(context); - auto realPath = CanonPath(state.ctx.paths.toRealPath(rewriteStrings(path.canonical().abs(), rewrites), context)); - - return flags.checkForPureEval - ? state.ctx.paths.checkSourcePath(realPath) - : realPath; + return checkFn(SourcePath(CanonPath( + state.ctx.paths.toRealPath(rewriteStrings(path.canonical().abs(), rewrites), context) + ))); } catch (Error & e) { e.addTrace(state.ctx.positions[pos], "while realising the context of path '%s'", path); throw; } } +static CheckedSourcePath realisePath(EvalState & state, const PosIdx pos, Value & v) +{ + return realisePath(state, pos, v, [&](auto p) { return state.ctx.paths.checkSourcePath(p); }); +} + /** * Add and attribute to the given attribute map from the output name to * the output path, or a placeholder. @@ -1198,7 +1196,7 @@ static void prim_pathExists(EvalState & state, const PosIdx pos, Value * * args, can’t just catch the exception here because we still want to throw if something in the evaluation of `arg` tries to access an unauthorized path). */ - auto path = realisePath(state, pos, arg, { .checkForPureEval = false }); + auto path = realisePath(state, pos, arg, std::identity{}); /* SourcePath doesn't know about trailing slash. */ auto mustBeDir = arg.type() == nString diff --git a/lix/libexpr/value-to-xml.cc b/lix/libexpr/value-to-xml.cc index 49c21623f..e0b4357ba 100644 --- a/lix/libexpr/value-to-xml.cc +++ b/lix/libexpr/value-to-xml.cc @@ -21,7 +21,7 @@ static void printValueAsXML(EvalState & state, bool strict, bool location, static void posToXML(EvalState & state, XMLAttrs & xmlAttrs, const Pos & pos) { - if (auto path = std::get_if(&pos.origin)) + if (auto path = std::get_if(&pos.origin)) xmlAttrs["path"] = path->to_string(); xmlAttrs["line"] = fmt("%1%", pos.line); xmlAttrs["column"] = fmt("%1%", pos.column); diff --git a/lix/libfetchers/fetch-to-store.cc b/lix/libfetchers/fetch-to-store.cc index ee7304338..a276a48fd 100644 --- a/lix/libfetchers/fetch-to-store.cc +++ b/lix/libfetchers/fetch-to-store.cc @@ -6,7 +6,7 @@ namespace nix { StorePath fetchToStore( Store & store, - const SourcePath & path, + const CheckedSourcePath & path, std::string_view name, FileIngestionMethod method, PathFilter * filter, diff --git a/lix/libfetchers/fetch-to-store.hh b/lix/libfetchers/fetch-to-store.hh index b0472dbaa..6d4d072aa 100644 --- a/lix/libfetchers/fetch-to-store.hh +++ b/lix/libfetchers/fetch-to-store.hh @@ -13,7 +13,7 @@ namespace nix { */ StorePath fetchToStore( Store & store, - const SourcePath & path, + const CheckedSourcePath & path, std::string_view name = "source", FileIngestionMethod method = FileIngestionMethod::Recursive, PathFilter * filter = nullptr, diff --git a/lix/libutil/position.cc b/lix/libutil/position.cc index cf2ff4b45..d98cd4e93 100644 --- a/lix/libutil/position.cc +++ b/lix/libutil/position.cc @@ -62,7 +62,7 @@ std::optional Pos::getSource() const // Get rid of the null terminators added by the parser. return std::string(s.source->c_str()); }, - [](const SourcePath & path) -> std::optional { + [](const CheckedSourcePath & path) -> std::optional { try { return path.readFile(); } catch (Error &) { diff --git a/lix/libutil/position.hh b/lix/libutil/position.hh index 3f89a0f5a..98fd7f15b 100644 --- a/lix/libutil/position.hh +++ b/lix/libutil/position.hh @@ -42,7 +42,7 @@ struct Pos auto operator<=>(const Hidden &) const = default; }; - typedef std::variant