forked from lix-project/lix
Disallow store path names that are . or .. (plus opt. -)
As discussed in the maintainer meeting on 2024-01-29.
Mainly this is to avoid a situation where the name is parsed and
treated as a file name, mostly to protect users.
.-* and ..-* are also considered invalid because they might strip
on that separator to remove versions. Doesn't really work, but that's
what we decided, and I won't argue with it, because .-* probably
doesn't seem to have a real world application anyway.
We do still permit a 1-character name that's just "-", which still
poses a similar risk in such a situation. We can't start disallowing
trailing -, because a non-zero number of users will need it and we've
seen how annoying and painful such a change is.
What matters most is preventing a situation where . or .. can be
injected, and to just get this done.
(cherry picked from commit f1b4663805a9dbcb1ace64ec110092d17c9155e0)
Change-Id: I900a8509933cee662f888c3c76fa8986b0058839
This commit is contained in:
parent
4b3dc66386
commit
b7ce11c97d
|
@ -5,6 +5,6 @@ prs: [9867, 9091, 9095, 9120, 9121, 9122, 9130, 9219, 9224]
|
||||||
---
|
---
|
||||||
|
|
||||||
Leading periods were allowed by accident in Nix 2.4. The Nix team has considered this to be a bug, but this behavior has since been relied on by users, leading to unnecessary difficulties.
|
Leading periods were allowed by accident in Nix 2.4. The Nix team has considered this to be a bug, but this behavior has since been relied on by users, leading to unnecessary difficulties.
|
||||||
From now on, leading periods are officially, definitively supported.
|
From now on, leading periods are officially, definitively supported. The names `.` and `..` are disallowed, as well as those starting with `.-` or `..-`.
|
||||||
|
|
||||||
Nix versions that denied leading periods are documented [in the issue](https://github.com/NixOS/nix/issues/912#issuecomment-1919583286).
|
Nix versions that denied leading periods are documented [in the issue](https://github.com/NixOS/nix/issues/912#issuecomment-1919583286).
|
||||||
|
|
|
@ -3,6 +3,11 @@
|
||||||
|
|
||||||
namespace nix {
|
namespace nix {
|
||||||
|
|
||||||
static constexpr std::string_view nameRegexStr = R"([0-9a-zA-Z\+\-\._\?=]+)";
|
|
||||||
|
static constexpr std::string_view nameRegexStr =
|
||||||
|
// This uses a negative lookahead: (?!\.\.?(-|$))
|
||||||
|
// - deny ".", "..", or those strings followed by '-'
|
||||||
|
// - when it's not those, start again at the start of the input and apply the next regex, which is [0-9a-zA-Z\+\-\._\?=]+
|
||||||
|
R"((?!\.\.?(-|$))[0-9a-zA-Z\+\-\._\?=]+)";
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
|
@ -12,6 +12,19 @@ static void checkName(std::string_view path, std::string_view name)
|
||||||
throw BadStorePath("store path '%s' has a name longer than %d characters",
|
throw BadStorePath("store path '%s' has a name longer than %d characters",
|
||||||
path, StorePath::MaxPathLen);
|
path, StorePath::MaxPathLen);
|
||||||
// See nameRegexStr for the definition
|
// See nameRegexStr for the definition
|
||||||
|
if (name[0] == '.') {
|
||||||
|
// check against "." and "..", followed by end or dash
|
||||||
|
if (name.size() == 1)
|
||||||
|
throw BadStorePath("store path '%s' has invalid name '%s'", path, name);
|
||||||
|
if (name[1] == '-')
|
||||||
|
throw BadStorePath("store path '%s' has invalid name '%s': first dash-separated component must not be '%s'", path, name, ".");
|
||||||
|
if (name[1] == '.') {
|
||||||
|
if (name.size() == 2)
|
||||||
|
throw BadStorePath("store path '%s' has invalid name '%s'", path, name);
|
||||||
|
if (name[2] == '-')
|
||||||
|
throw BadStorePath("store path '%s' has invalid name '%s': first dash-separated component must not be '%s'", path, name, "..");
|
||||||
|
}
|
||||||
|
}
|
||||||
for (auto c : name)
|
for (auto c : name)
|
||||||
if (!((c >= '0' && c <= '9')
|
if (!((c >= '0' && c <= '9')
|
||||||
|| (c >= 'a' && c <= 'z')
|
|| (c >= 'a' && c <= 'z')
|
||||||
|
|
|
@ -39,6 +39,12 @@ TEST_DONT_PARSE(double_star, "**")
|
||||||
TEST_DONT_PARSE(star_first, "*,foo")
|
TEST_DONT_PARSE(star_first, "*,foo")
|
||||||
TEST_DONT_PARSE(star_second, "foo,*")
|
TEST_DONT_PARSE(star_second, "foo,*")
|
||||||
TEST_DONT_PARSE(bang, "foo!o")
|
TEST_DONT_PARSE(bang, "foo!o")
|
||||||
|
TEST_DONT_PARSE(dot, ".")
|
||||||
|
TEST_DONT_PARSE(dot_dot, "..")
|
||||||
|
TEST_DONT_PARSE(dot_dot_dash, "..-1")
|
||||||
|
TEST_DONT_PARSE(dot_dash, ".-1")
|
||||||
|
TEST_DONT_PARSE(dot_dot_dash_a, "..-a")
|
||||||
|
TEST_DONT_PARSE(dot_dash_a, ".-a")
|
||||||
|
|
||||||
#undef TEST_DONT_PARSE
|
#undef TEST_DONT_PARSE
|
||||||
|
|
||||||
|
@ -63,6 +69,10 @@ TEST_DO_PARSE(period, "foo.txt")
|
||||||
TEST_DO_PARSE(question_mark, "foo?why")
|
TEST_DO_PARSE(question_mark, "foo?why")
|
||||||
TEST_DO_PARSE(equals_sign, "foo=foo")
|
TEST_DO_PARSE(equals_sign, "foo=foo")
|
||||||
TEST_DO_PARSE(dotfile, ".gitignore")
|
TEST_DO_PARSE(dotfile, ".gitignore")
|
||||||
|
TEST_DO_PARSE(triple_dot_a, "...a")
|
||||||
|
TEST_DO_PARSE(triple_dot_1, "...1")
|
||||||
|
TEST_DO_PARSE(triple_dot_dash, "...-")
|
||||||
|
TEST_DO_PARSE(triple_dot, "...")
|
||||||
|
|
||||||
#undef TEST_DO_PARSE
|
#undef TEST_DO_PARSE
|
||||||
|
|
||||||
|
@ -84,6 +94,64 @@ RC_GTEST_FIXTURE_PROP(
|
||||||
RC_ASSERT(p == store->parseStorePath(store->printStorePath(p)));
|
RC_ASSERT(p == store->parseStorePath(store->printStorePath(p)));
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
||||||
|
RC_GTEST_FIXTURE_PROP(
|
||||||
|
StorePathTest,
|
||||||
|
prop_check_regex_eq_parse,
|
||||||
|
())
|
||||||
|
{
|
||||||
|
static auto nameFuzzer =
|
||||||
|
rc::gen::container<std::string>(
|
||||||
|
rc::gen::oneOf(
|
||||||
|
// alphanum, repeated to weigh heavier
|
||||||
|
rc::gen::oneOf(
|
||||||
|
rc::gen::inRange('0', '9'),
|
||||||
|
rc::gen::inRange('a', 'z'),
|
||||||
|
rc::gen::inRange('A', 'Z')
|
||||||
|
),
|
||||||
|
// valid symbols
|
||||||
|
rc::gen::oneOf(
|
||||||
|
rc::gen::just('+'),
|
||||||
|
rc::gen::just('-'),
|
||||||
|
rc::gen::just('.'),
|
||||||
|
rc::gen::just('_'),
|
||||||
|
rc::gen::just('?'),
|
||||||
|
rc::gen::just('=')
|
||||||
|
),
|
||||||
|
// symbols for scary .- and ..- cases, repeated for weight
|
||||||
|
rc::gen::just('.'), rc::gen::just('.'),
|
||||||
|
rc::gen::just('.'), rc::gen::just('.'),
|
||||||
|
rc::gen::just('-'), rc::gen::just('-'),
|
||||||
|
// ascii symbol ranges
|
||||||
|
rc::gen::oneOf(
|
||||||
|
rc::gen::inRange(' ', '/'),
|
||||||
|
rc::gen::inRange(':', '@'),
|
||||||
|
rc::gen::inRange('[', '`'),
|
||||||
|
rc::gen::inRange('{', '~')
|
||||||
|
),
|
||||||
|
// typical whitespace
|
||||||
|
rc::gen::oneOf(
|
||||||
|
rc::gen::just(' '),
|
||||||
|
rc::gen::just('\t'),
|
||||||
|
rc::gen::just('\n'),
|
||||||
|
rc::gen::just('\r')
|
||||||
|
),
|
||||||
|
// some chance of control codes, non-ascii or other garbage we missed
|
||||||
|
rc::gen::inRange('\0', '\xff')
|
||||||
|
));
|
||||||
|
|
||||||
|
auto name = *nameFuzzer;
|
||||||
|
|
||||||
|
std::string path = store->storeDir + "/575s52sh487i0ylmbs9pvi606ljdszr0-" + name;
|
||||||
|
bool parsed = false;
|
||||||
|
try {
|
||||||
|
store->parseStorePath(path);
|
||||||
|
parsed = true;
|
||||||
|
} catch (const BadStorePath &) {
|
||||||
|
}
|
||||||
|
RC_ASSERT(parsed == std::regex_match(std::string { name }, nameRegex));
|
||||||
|
}
|
||||||
|
|
||||||
#endif
|
#endif
|
||||||
|
|
||||||
}
|
}
|
||||||
|
|
Loading…
Reference in a new issue