Merge pull request #5279 from edolstra/restrict-path-inputs

Fix relative path input handling
This commit is contained in:
Eelco Dolstra 2021-09-21 14:52:30 +02:00 committed by GitHub
commit c81f9761cc
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 34 additions and 19 deletions

View file

@ -89,10 +89,12 @@ static void expectType(EvalState & state, ValueType type,
}
static std::map<FlakeId, FlakeInput> parseFlakeInputs(
EvalState & state, Value * value, const Pos & pos);
EvalState & state, Value * value, const Pos & pos,
const std::optional<Path> & baseDir);
static FlakeInput parseFlakeInput(EvalState & state,
const std::string & inputName, Value * value, const Pos & pos)
const std::string & inputName, Value * value, const Pos & pos,
const std::optional<Path> & baseDir)
{
expectType(state, nAttrs, *value, pos);
@ -116,7 +118,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
expectType(state, nBool, *attr.value, *attr.pos);
input.isFlake = attr.value->boolean;
} else if (attr.name == sInputs) {
input.overrides = parseFlakeInputs(state, attr.value, *attr.pos);
input.overrides = parseFlakeInputs(state, attr.value, *attr.pos, baseDir);
} else if (attr.name == sFollows) {
expectType(state, nString, *attr.value, *attr.pos);
input.follows = parseInputPath(attr.value->string.s);
@ -154,7 +156,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
if (!attrs.empty())
throw Error("unexpected flake input attribute '%s', at %s", attrs.begin()->first, pos);
if (url)
input.ref = parseFlakeRef(*url, {}, true);
input.ref = parseFlakeRef(*url, baseDir, true);
}
if (!input.follows && !input.ref)
@ -164,7 +166,8 @@ static FlakeInput parseFlakeInput(EvalState & state,
}
static std::map<FlakeId, FlakeInput> parseFlakeInputs(
EvalState & state, Value * value, const Pos & pos)
EvalState & state, Value * value, const Pos & pos,
const std::optional<Path> & baseDir)
{
std::map<FlakeId, FlakeInput> inputs;
@ -175,7 +178,8 @@ static std::map<FlakeId, FlakeInput> parseFlakeInputs(
parseFlakeInput(state,
inputAttr.name,
inputAttr.value,
*inputAttr.pos));
*inputAttr.pos,
baseDir));
}
return inputs;
@ -191,7 +195,8 @@ static Flake getFlake(
state, originalRef, allowLookup, flakeCache);
// Guard against symlink attacks.
auto flakeFile = canonPath(sourceInfo.actualPath + "/" + lockedRef.subdir + "/flake.nix");
auto flakeDir = canonPath(sourceInfo.actualPath + "/" + lockedRef.subdir);
auto flakeFile = canonPath(flakeDir + "/flake.nix");
if (!isInDir(flakeFile, sourceInfo.actualPath))
throw Error("'flake.nix' file of flake '%s' escapes from '%s'",
lockedRef, state.store->printStorePath(sourceInfo.storePath));
@ -219,7 +224,7 @@ static Flake getFlake(
auto sInputs = state.symbols.create("inputs");
if (auto inputs = vInfo.attrs->get(sInputs))
flake.inputs = parseFlakeInputs(state, inputs->value, *inputs->pos);
flake.inputs = parseFlakeInputs(state, inputs->value, *inputs->pos, flakeDir);
auto sOutputs = state.symbols.create("outputs");
@ -488,10 +493,8 @@ LockedFlake lockFlake(
// If this input is a path, recurse it down.
// This allows us to resolve path inputs relative to the current flake.
if (localRef.input.getType() == "path") {
localRef.input.parent = parentPath;
localPath = canonPath(parentPath + "/" + *input.ref->input.getSourcePath());
}
if (localRef.input.getType() == "path")
localPath = absPath(*input.ref->input.getSourcePath(), parentPath);
auto inputFlake = getFlake(state, localRef, useRegistries, flakeCache);

View file

@ -172,8 +172,12 @@ std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(
auto parsedURL = parseURL(url);
std::string fragment;
std::swap(fragment, parsedURL.fragment);
auto input = Input::fromURL(parsedURL);
input.parent = baseDir;
return std::make_pair(
FlakeRef(Input::fromURL(parsedURL), get(parsedURL.query, "dir").value_or("")),
FlakeRef(std::move(input), get(parsedURL.query, "dir").value_or("")),
fragment);
}
}

View file

@ -85,18 +85,26 @@ struct PathInputScheme : InputScheme
std::string absPath;
auto path = getStrAttr(input.attrs, "path");
if (path[0] != '/' && input.parent) {
if (path[0] != '/') {
if (!input.parent)
throw Error("cannot fetch input '%s' because it uses a relative path", input.to_string());
auto parent = canonPath(*input.parent);
// the path isn't relative, prefix it
absPath = canonPath(parent + "/" + path);
absPath = nix::absPath(path, parent);
// for security, ensure that if the parent is a store path, it's inside it
if (!parent.rfind(store->storeDir, 0) && absPath.rfind(store->storeDir, 0))
throw BadStorePath("relative path '%s' points outside of its parent's store path %s, this is a security violation", path, parent);
if (store->isInStore(parent)) {
auto storePath = store->printStorePath(store->toStorePath(parent).first);
if (!isInDir(absPath, storePath))
throw BadStorePath("relative path '%s' points outside of its parent's store path '%s'", path, storePath);
}
} else
absPath = path;
Activity act(*logger, lvlTalkative, actUnknown, fmt("copying '%s'", absPath));
// FIXME: check whether access to 'path' is allowed.
auto storePath = store->maybeParseStorePath(absPath);

View file

@ -766,7 +766,7 @@ cat > $flakeFollowsA/flake.nix <<EOF
{
description = "Flake A";
inputs = {
B.url = "path:./../../flakeB";
B.url = "path:../flakeB";
};
outputs = { ... }: {};
}
@ -774,7 +774,7 @@ EOF
git -C $flakeFollowsA add flake.nix
nix flake lock $flakeFollowsA 2>&1 | grep 'this is a security violation'
nix flake lock $flakeFollowsA 2>&1 | grep 'points outside'
# Test flake in store does not evaluate
rm -rf $badFlakeDir