diff options
author | aszlig <aszlig@nix.build> | 2018-08-03T04·24+0200 |
---|---|---|
committer | aszlig <aszlig@nix.build> | 2018-08-03T04·46+0200 |
commit | 43e28a1b756c2f7ee139c999e6169a71f555e9e5 (patch) | |
tree | 93d5baec133de00ce914d71f1a5688f0d1eee954 | |
parent | aa64e95bc82b3a57f3a645a746aacf4d2479266e (diff) |
Fix symlink leak in restricted eval mode
In EvalState::checkSourcePath, the path is checked against the list of allowed paths first and later it's checked again *after* resolving symlinks. The resolving of the symlinks is done via canonPath, which also strips out "../" and "./". However after the canonicalisation the error message pointing out that the path is not allowed prints the symlink target in the error message. Even if we'd suppress the message, symlink targets could still be leaked if the symlink target doesn't exist (in this case the error is thrown in canonPath). So instead, we now do canonPath() without symlink resolving first before even checking against the list of allowed paths and then later do the symlink resolving and checking the allowed paths again. The first call to canonPath() should get rid of all the "../" and "./", so in theory the only way to leak a symlink if the attacker is able to put a symlink in one of the paths allowed by restricted evaluation mode. For the latter I don't think this is part of the threat model, because if the attacker can write to that path, the attack vector is even larger. Signed-off-by: aszlig <aszlig@nix.build>
-rw-r--r-- | src/libexpr/eval.cc | 14 | ||||
-rw-r--r-- | tests/restricted.sh | 11 |
2 files changed, 21 insertions, 4 deletions
diff --git a/src/libexpr/eval.cc b/src/libexpr/eval.cc index e09297546c95..3abde6c92961 100644 --- a/src/libexpr/eval.cc +++ b/src/libexpr/eval.cc @@ -349,19 +349,25 @@ Path EvalState::checkSourcePath(const Path & path_) bool found = false; + /* First canonicalize the path without symlinks, so we make sure an + * attacker can't append ../../... to a path that would be in allowedPaths + * and thus leak symlink targets. + */ + Path abspath = canonPath(path_); + for (auto & i : *allowedPaths) { - if (isDirOrInDir(path_, i)) { + if (isDirOrInDir(abspath, i)) { found = true; break; } } if (!found) - throw RestrictedPathError("access to path '%1%' is forbidden in restricted mode", path_); + throw RestrictedPathError("access to path '%1%' is forbidden in restricted mode", abspath); /* Resolve symlinks. */ - debug(format("checking access to '%s'") % path_); - Path path = canonPath(path_, true); + debug(format("checking access to '%s'") % abspath); + Path path = canonPath(abspath, true); for (auto & i : *allowedPaths) { if (isDirOrInDir(path, i)) { diff --git a/tests/restricted.sh b/tests/restricted.sh index a87d8ec2c940..e02becc60e38 100644 --- a/tests/restricted.sh +++ b/tests/restricted.sh @@ -38,3 +38,14 @@ ln -sfn $(pwd)/restricted.nix $TEST_ROOT/restricted.nix nix-instantiate --eval --restrict-eval $TEST_ROOT/restricted.nix -I $TEST_ROOT -I . [[ $(nix eval --raw --restrict-eval -I . '(builtins.readFile "${import ./simple.nix}/hello")') == 'Hello World!' ]] + +# Check whether we can leak symlink information through directory traversal. +traverseDir="$(pwd)/restricted-traverse-me" +ln -sfn "$(pwd)/restricted-secret" "$(pwd)/restricted-innocent" +mkdir -p "$traverseDir" +goUp="..$(echo "$traverseDir" | sed -e 's,[^/]\+,..,g')" +output="$(nix eval --raw --restrict-eval -I "$traverseDir" \ + "(builtins.readFile \"$traverseDir/$goUp$(pwd)/restricted-innocent\")" \ + 2>&1 || :)" +echo "$output" | grep "is forbidden" +! echo "$output" | grep -F restricted-secret |