diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index 648032ea331..4ac4e18dbf2 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -395,12 +395,14 @@ EvalState::EvalState( , emptyBindings(0) , rootFS( evalSettings.restrictEval || evalSettings.pureEval - ? ref(AllowListInputAccessor::create(makeFSInputAccessor(), {}, + ? ref(AllowListInputAccessor::create(makeFSInputAccessor(), {}, {}, [](const CanonPath & path) -> RestrictedPathError { - auto modeInformation = evalSettings.pureEval - ? "in pure evaluation mode (use '--impure' to override)" - : "in restricted mode"; - throw RestrictedPathError("access to absolute path '%1%' is forbidden %2%", path, modeInformation); + throw RestrictedPathError( + std::string("access to absolute path '%1%' is forbidden ") + + (evalSettings.pureEval + ? "in pure evaluation mode (use '--impure' to override)" + : "in restricted mode"), + path); })) : makeFSInputAccessor()) , corepkgsFS(makeMemoryInputAccessor()) diff --git a/src/libfetchers/filtering-input-accessor.cc b/src/libfetchers/filtering-input-accessor.cc index 32343abc4f0..a14deb0db53 100644 --- a/src/libfetchers/filtering-input-accessor.cc +++ b/src/libfetchers/filtering-input-accessor.cc @@ -1,5 +1,7 @@ #include "filtering-input-accessor.hh" +#include + namespace nix { std::string FilteringInputAccessor::readFile(const CanonPath & path) @@ -13,10 +15,15 @@ bool FilteringInputAccessor::pathExists(const CanonPath & path) return isAllowed(path) && next->pathExists(prefix / path); } -std::optional FilteringInputAccessor::maybeLstat(const CanonPath & path) +SourceAccessor::Stat FilteringInputAccessor::lstat(const CanonPath & path) { checkAccess(path); - return next->maybeLstat(prefix / path); + return next->lstat(prefix / path); +} + +std::optional FilteringInputAccessor::maybeLstat(const CanonPath & path) +{ + return isAllowed(path) ? next->maybeLstat(prefix / path) : std::nullopt; } InputAccessor::DirEntries FilteringInputAccessor::readDirectory(const CanonPath & path) @@ -51,33 +58,46 @@ void FilteringInputAccessor::checkAccess(const CanonPath & path) struct AllowListInputAccessorImpl : AllowListInputAccessor { - std::set allowedPrefixes; + std::unordered_set allowedPrefixes; + std::unordered_set allowedPaths; AllowListInputAccessorImpl( ref next, - std::set && allowedPrefixes, + std::unordered_set && allowedPrefixes, + std::unordered_set && allowedPaths, MakeNotAllowedError && makeNotAllowedError) : AllowListInputAccessor(SourcePath(next), std::move(makeNotAllowedError)) , allowedPrefixes(std::move(allowedPrefixes)) + , allowedPaths(std::move(allowedPaths)) { } bool isAllowed(const CanonPath & path) override { - return path.isAllowed(allowedPrefixes); + return allowedPaths.count(path) || path.isAllowed(allowedPrefixes); } void allowPrefix(CanonPath prefix) override { allowedPrefixes.insert(std::move(prefix)); } + + void allowPath(CanonPath path) override + { + allowedPaths.insert(std::move(path)); + } }; ref AllowListInputAccessor::create( ref next, - std::set && allowedPrefixes, + std::unordered_set && allowedPrefixes, + std::unordered_set && allowedPaths, MakeNotAllowedError && makeNotAllowedError) { - return make_ref(next, std::move(allowedPrefixes), std::move(makeNotAllowedError)); + return make_ref( + next, + std::move(allowedPrefixes), + std::move(allowedPaths), + std::move(makeNotAllowedError)); } bool CachingFilteringInputAccessor::isAllowed(const CanonPath & path) diff --git a/src/libfetchers/filtering-input-accessor.hh b/src/libfetchers/filtering-input-accessor.hh index 8111a72c531..33e31ed3838 100644 --- a/src/libfetchers/filtering-input-accessor.hh +++ b/src/libfetchers/filtering-input-accessor.hh @@ -33,6 +33,8 @@ struct FilteringInputAccessor : InputAccessor bool pathExists(const CanonPath & path) override; + Stat lstat(const CanonPath & path) override; + std::optional maybeLstat(const CanonPath & path) override; DirEntries readDirectory(const CanonPath & path) override; @@ -60,13 +62,20 @@ struct FilteringInputAccessor : InputAccessor struct AllowListInputAccessor : public FilteringInputAccessor { /** - * Grant access to the specified prefix. + * Grant access to the specified prefix, i.e. the path *and* its + * children. */ virtual void allowPrefix(CanonPath prefix) = 0; + /** + * Grant access to a path but not its children. + */ + virtual void allowPath(CanonPath path) = 0; + static ref create( ref next, - std::set && allowedPrefixes, + std::unordered_set && allowedPrefixes, + std::unordered_set && allowedPaths, MakeNotAllowedError && makeNotAllowedError); using FilteringInputAccessor::FilteringInputAccessor; diff --git a/src/libfetchers/git-utils.cc b/src/libfetchers/git-utils.cc index 5ecd825b7af..a4b58c8ab53 100644 --- a/src/libfetchers/git-utils.cc +++ b/src/libfetchers/git-utils.cc @@ -943,17 +943,23 @@ ref GitRepoImpl::getAccessor(const Hash & rev, bool exportIgnore) ref GitRepoImpl::getAccessor(const WorkdirInfo & wd, bool exportIgnore, MakeNotAllowedError makeNotAllowedError) { auto self = ref(shared_from_this()); - /* In case of an empty workdir, return an empty in-memory tree. We - cannot use AllowListInputAccessor because it would return an - error for the root (and we can't add the root to the allow-list - since that would allow access to all its children). */ - ref fileAccessor = - wd.files.empty() - ? makeEmptyInputAccessor() - : AllowListInputAccessor::create( - makeFSInputAccessor(path), - std::set { wd.files }, - std::move(makeNotAllowedError)).cast(); + + /* Grant access to the parent directories of every accessible + file. The root is always accessible. */ + std::unordered_set files{CanonPath::root}; + for (auto path : wd.files) { + while (!path.isRoot()) { + if (!files.insert(path).second) break; + path.pop(); + } + } + + auto fileAccessor = AllowListInputAccessor::create( + makeFSInputAccessor(path), + {}, + std::move(files), + std::move(makeNotAllowedError)); + if (exportIgnore) return make_ref(self, fileAccessor, std::nullopt); else diff --git a/src/libutil/canon-path.cc b/src/libutil/canon-path.cc index 27f048697e5..b7b62561982 100644 --- a/src/libutil/canon-path.cc +++ b/src/libutil/canon-path.cc @@ -89,25 +89,16 @@ CanonPath CanonPath::operator / (std::string_view c) const return res; } -bool CanonPath::isAllowed(const std::set & allowed) const +bool CanonPath::isAllowed(const std::unordered_set & allowed) const { - /* Check if `this` is an exact match or the parent of an - allowed path. */ - auto lb = allowed.lower_bound(*this); - if (lb != allowed.end()) { - if (lb->isWithin(*this)) - return true; - } - - /* Check if a parent of `this` is allowed. */ auto path = *this; - while (!path.isRoot()) { - path.pop(); + while (true) { if (allowed.count(path)) return true; + if (path.isRoot()) + return false; + path.pop(); } - - return false; } std::ostream & operator << (std::ostream & stream, const CanonPath & path) diff --git a/src/libutil/canon-path.hh b/src/libutil/canon-path.hh index 8f5a1c2793d..54703fa7a9b 100644 --- a/src/libutil/canon-path.hh +++ b/src/libutil/canon-path.hh @@ -5,7 +5,7 @@ #include #include #include -#include +#include #include namespace nix { @@ -210,12 +210,10 @@ public: CanonPath operator / (std::string_view c) const; /** - * Check whether access to this path is allowed, which is the case - * if 1) `this` is within any of the `allowed` paths; or 2) any of - * the `allowed` paths are within `this`. (The latter condition - * ensures access to the parents of allowed paths.) + * Check whether access to this path is allowed, i.e. `this` is + * within any of the `allowed` paths. */ - bool isAllowed(const std::set & allowed) const; + bool isAllowed(const std::unordered_set & allowed) const; /** * Return a representation `x` of `path` relative to `this`, i.e. diff --git a/src/libutil/source-accessor.hh b/src/libutil/source-accessor.hh index 1f272327f81..4725af3e6d2 100644 --- a/src/libutil/source-accessor.hh +++ b/src/libutil/source-accessor.hh @@ -111,7 +111,7 @@ struct SourceAccessor std::optional narOffset; }; - Stat lstat(const CanonPath & path); + virtual Stat lstat(const CanonPath & path); virtual std::optional maybeLstat(const CanonPath & path) = 0; diff --git a/tests/functional/flakes/flakes.sh b/tests/functional/flakes/flakes.sh index 4f41cae0aef..e7c5007e2de 100644 --- a/tests/functional/flakes/flakes.sh +++ b/tests/functional/flakes/flakes.sh @@ -34,6 +34,7 @@ cat > "$flake2Dir/flake.nix" <' | grepQuiet "forbidden in restricted mode" @@ -20,21 +20,21 @@ nix-instantiate --restrict-eval --eval -E 'let __nixPath = [ { prefix = "foo"; p p=$(nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)") cmp $p restricted.sh -(! nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval) +expectStderr 1 nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval | grepQuiet "forbidden in restricted mode" -(! nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)/restricted.sh/") +expectStderr 1 nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)/restricted.sh/" | grepQuiet "forbidden in restricted mode" nix eval --raw --expr "builtins.fetchurl file://$(pwd)/restricted.sh" --impure --restrict-eval --allowed-uris "file://$(pwd)/restricted.sh" -(! nix eval --raw --expr "builtins.fetchurl https://github.com/NixOS/patchelf/archive/master.tar.gz" --impure --restrict-eval) -(! nix eval --raw --expr "builtins.fetchTarball https://github.com/NixOS/patchelf/archive/master.tar.gz" --impure --restrict-eval) -(! nix eval --raw --expr "fetchGit git://github.com/NixOS/patchelf.git" --impure --restrict-eval) +expectStderr 1 nix eval --raw --expr "builtins.fetchurl https://github.com/NixOS/patchelf/archive/master.tar.gz" --impure --restrict-eval | grepQuiet "forbidden in restricted mode" +expectStderr 1 nix eval --raw --expr "builtins.fetchTarball https://github.com/NixOS/patchelf/archive/master.tar.gz" --impure --restrict-eval | grepQuiet "forbidden in restricted mode" +expectStderr 1 nix eval --raw --expr "fetchGit git://github.com/NixOS/patchelf.git" --impure --restrict-eval | grepQuiet "forbidden in restricted mode" ln -sfn $(pwd)/restricted.nix $TEST_ROOT/restricted.nix [[ $(nix-instantiate --eval $TEST_ROOT/restricted.nix) == 3 ]] -(! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix) -(! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT) -(! nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I .) +expectStderr 1 nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix | grepQuiet "forbidden in restricted mode" +expectStderr 1 nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT | grepQuiet "forbidden in restricted mode" +expectStderr 1 nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I . | grepQuiet "forbidden in restricted mode" nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT -I . [[ $(nix eval --raw --impure --restrict-eval -I . --expr 'builtins.readFile "${import ./simple.nix}/hello"') == 'Hello World!' ]] @@ -48,8 +48,8 @@ expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { pr expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir " -I $TEST_ROOT/tunnel.d | grepQuiet "forbidden in restricted mode" -# Reading the parents of allowed paths should show only the ancestors of the allowed paths. -[[ $(nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir " -I $TEST_ROOT/tunnel.d) == '{ "tunnel.d" = "directory"; }' ]] +# Reading the parents of allowed paths is forbidden. +expectStderr 1 nix-instantiate --restrict-eval --eval -E "let __nixPath = [ { prefix = \"foo\"; path = $TEST_ROOT/tunnel.d; } ]; in builtins.readDir " -I $TEST_ROOT/tunnel.d | grepQuiet "forbidden in restricted mode" # Check whether we can leak symlink information through directory traversal. traverseDir="$(pwd)/restricted-traverse-me" @@ -61,5 +61,3 @@ output="$(nix eval --raw --restrict-eval -I "$traverseDir" \ 2>&1 || :)" echo "$output" | grep "is forbidden" echo "$output" | grepInverse -F restricted-secret - -expectStderr 1 nix-instantiate --restrict-eval true ./dependencies.nix | grepQuiet "forbidden in restricted mode" diff --git a/tests/unit/libutil/canon-path.cc b/tests/unit/libutil/canon-path.cc index 7f91308afe1..a47db2056c3 100644 --- a/tests/unit/libutil/canon-path.cc +++ b/tests/unit/libutil/canon-path.cc @@ -143,7 +143,7 @@ namespace nix { } TEST(CanonPath, allowed) { - std::set allowed { + std::unordered_set allowed { CanonPath("foo/bar"), CanonPath("foo!"), CanonPath("xyzzy"), @@ -152,11 +152,11 @@ namespace nix { ASSERT_TRUE (CanonPath("foo/bar").isAllowed(allowed)); ASSERT_TRUE (CanonPath("foo/bar/bla").isAllowed(allowed)); - ASSERT_TRUE (CanonPath("foo").isAllowed(allowed)); + ASSERT_FALSE(CanonPath("foo").isAllowed(allowed)); ASSERT_FALSE(CanonPath("bar").isAllowed(allowed)); ASSERT_FALSE(CanonPath("bar/a").isAllowed(allowed)); - ASSERT_TRUE (CanonPath("a").isAllowed(allowed)); - ASSERT_TRUE (CanonPath("a/b").isAllowed(allowed)); + ASSERT_FALSE(CanonPath("a").isAllowed(allowed)); + ASSERT_FALSE(CanonPath("a/b").isAllowed(allowed)); ASSERT_TRUE (CanonPath("a/b/c").isAllowed(allowed)); ASSERT_TRUE (CanonPath("a/b/c/d").isAllowed(allowed)); ASSERT_TRUE (CanonPath("a/b/c/d/e").isAllowed(allowed)); @@ -164,7 +164,7 @@ namespace nix { ASSERT_FALSE(CanonPath("a/b/d").isAllowed(allowed)); ASSERT_FALSE(CanonPath("aaa").isAllowed(allowed)); ASSERT_FALSE(CanonPath("zzz").isAllowed(allowed)); - ASSERT_TRUE (CanonPath("/").isAllowed(allowed)); + ASSERT_FALSE(CanonPath("/").isAllowed(allowed)); } TEST(CanonPath, makeRelative) {