diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index e26b7eb93..20d8a1e09 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -82,7 +82,7 @@ void SourceAccessor::dumpPath( name.erase(pos); } if (!unhacked.emplace(name, i.first).second) - throw Error("file name collision in between '%s' and '%s'", + throw Error("file name collision between '%s' and '%s'", (path / unhacked[name]), (path / i.first)); } else @@ -128,9 +128,10 @@ void dumpString(std::string_view s, Sink & sink) } -static SerialisationError badArchive(const std::string & s) +template +static SerialisationError badArchive(std::string_view s, const Args & ... args) { - return SerialisationError("bad archive: " + s); + return SerialisationError("bad archive: " + s, args...); } @@ -167,120 +168,97 @@ struct CaseInsensitiveCompare static void parse(FileSystemObjectSink & sink, Source & source, const CanonPath & path) { - std::string s; - - s = readString(source); - if (s != "(") throw badArchive("expected open tag"); - - std::map names; - auto getString = [&]() { checkInterrupt(); return readString(source); }; - // For first iteration - s = getString(); + auto expectTag = [&](std::string_view expected) { + auto tag = getString(); + if (tag != expected) + throw badArchive("expected tag '%s', got '%s'", expected, tag); + }; - while (1) { + expectTag("("); - if (s == ")") { - break; - } + expectTag("type"); - else if (s == "type") { - std::string t = getString(); + auto type = getString(); - if (t == "regular") { - sink.createRegularFile(path, [&](auto & crf) { - while (1) { - s = getString(); + if (type == "regular") { + sink.createRegularFile(path, [&](auto & crf) { + auto tag = getString(); - if (s == "contents") { - parseContents(crf, source); - } - - else if (s == "executable") { - auto s2 = getString(); - if (s2 != "") throw badArchive("executable marker has non-empty value"); - crf.isExecutable(); - } - - else break; - } - }); + if (tag == "executable") { + auto s2 = getString(); + if (s2 != "") throw badArchive("executable marker has non-empty value"); + crf.isExecutable(); + tag = getString(); } - else if (t == "directory") { - sink.createDirectory(path); + if (tag == "contents") + parseContents(crf, source); - std::string prevName; - - while (1) { - s = getString(); - - if (s == "entry") { - std::string name; - - s = getString(); - if (s != "(") throw badArchive("expected open tag"); - - while (1) { - s = getString(); - - if (s == ")") { - break; - } else if (s == "name") { - name = getString(); - if (name.empty() || name == "." || name == ".." || name.find('/') != std::string::npos || name.find((char) 0) != std::string::npos) - throw Error("NAR contains invalid file name '%1%'", name); - if (name <= prevName) - throw Error("NAR directory is not sorted"); - prevName = name; - if (archiveSettings.useCaseHack) { - auto i = names.find(name); - if (i != names.end()) { - debug("case collision between '%1%' and '%2%'", i->first, name); - name += caseHackSuffix; - name += std::to_string(++i->second); - auto j = names.find(name); - if (j != names.end()) - throw Error("NAR contains file name '%s' that collides with case-hacked file name '%s'", prevName, j->first); - } else - names[name] = 0; - } - } else if (s == "node") { - if (name.empty()) throw badArchive("entry name missing"); - parse(sink, source, path / name); - } else - throw badArchive("unknown field " + s); - } - } - - else break; - } - } - - else if (t == "symlink") { - s = getString(); - - if (s != "target") - throw badArchive("expected 'target' got " + s); - - std::string target = getString(); - sink.createSymlink(path, target); - - // for the next iteration - s = getString(); - } - - else throw badArchive("unknown file type " + t); - - } - - else - throw badArchive("unknown field " + s); + expectTag(")"); + }); } + + else if (type == "directory") { + sink.createDirectory(path); + + std::map names; + + std::string prevName; + + while (1) { + auto tag = getString(); + + if (tag == ")") break; + + if (tag != "entry") + throw badArchive("expected tag 'entry' or ')', got '%s'", tag); + + expectTag("("); + + expectTag("name"); + + auto name = getString(); + if (name.empty() || name == "." || name == ".." || name.find('/') != std::string::npos || name.find((char) 0) != std::string::npos) + throw badArchive("NAR contains invalid file name '%1%'", name); + if (name <= prevName) + throw badArchive("NAR directory is not sorted"); + prevName = name; + if (archiveSettings.useCaseHack) { + auto i = names.find(name); + if (i != names.end()) { + debug("case collision between '%1%' and '%2%'", i->first, name); + name += caseHackSuffix; + name += std::to_string(++i->second); + auto j = names.find(name); + if (j != names.end()) + throw badArchive("NAR contains file name '%s' that collides with case-hacked file name '%s'", prevName, j->first); + } else + names[name] = 0; + } + + expectTag("node"); + + parse(sink, source, path / name); + + expectTag(")"); + } + } + + else if (type == "symlink") { + expectTag("target"); + + auto target = getString(); + sink.createSymlink(path, target); + + expectTag(")"); + } + + else throw badArchive("unknown file type '%s'", type); } diff --git a/tests/functional/dot.nar b/tests/functional/dot.nar new file mode 100644 index 000000000..3a9452f67 Binary files /dev/null and b/tests/functional/dot.nar differ diff --git a/tests/functional/dotdot.nar b/tests/functional/dotdot.nar new file mode 100644 index 000000000..f8d019c39 Binary files /dev/null and b/tests/functional/dotdot.nar differ diff --git a/tests/functional/empty.nar b/tests/functional/empty.nar new file mode 100644 index 000000000..43434f2b4 Binary files /dev/null and b/tests/functional/empty.nar differ diff --git a/tests/functional/executable-after-contents.nar b/tests/functional/executable-after-contents.nar new file mode 100644 index 000000000..f8c003480 Binary files /dev/null and b/tests/functional/executable-after-contents.nar differ diff --git a/tests/functional/name-after-node.nar b/tests/functional/name-after-node.nar new file mode 100644 index 000000000..3f5cd9d0b Binary files /dev/null and b/tests/functional/name-after-node.nar differ diff --git a/tests/functional/nars.sh b/tests/functional/nars.sh index 9f5f43dc6..28876e497 100755 --- a/tests/functional/nars.sh +++ b/tests/functional/nars.sh @@ -67,6 +67,12 @@ expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | gre rm -rf "$TEST_ROOT/case" opts=("--option" "use-case-hack" "true") nix-store "${opts[@]}" --restore "$TEST_ROOT/case" < case.nar +[[ -e "$TEST_ROOT/case/xt_CONNMARK.h" ]] +[[ -e "$TEST_ROOT/case/xt_CONNmark.h~nix~case~hack~1" ]] +[[ -e "$TEST_ROOT/case/xt_connmark.h~nix~case~hack~2" ]] +[[ -e "$TEST_ROOT/case/x/FOO" ]] +[[ -d "$TEST_ROOT/case/x/Foo~nix~case~hack~1" ]] +[[ -e "$TEST_ROOT/case/x/foo~nix~case~hack~2/a~nix~case~hack~1/foo" ]] nix-store "${opts[@]}" --dump "$TEST_ROOT/case" > "$TEST_ROOT/case.nar" cmp case.nar "$TEST_ROOT/case.nar" [ "$(nix-hash "${opts[@]}" --type sha256 "$TEST_ROOT/case")" = "$(nix-hash --flat --type sha256 case.nar)" ] @@ -92,3 +98,31 @@ else [[ -e $TEST_ROOT/out/â ]] [[ -e $TEST_ROOT/out/â ]] fi + +# Unpacking a NAR with a NUL character in a file name should fail. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < nul.nar | grepQuiet "NAR contains invalid file name 'f" + +# Likewise for a '.' filename. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < dot.nar | grepQuiet "NAR contains invalid file name '.'" + +# Likewise for a '..' filename. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < dotdot.nar | grepQuiet "NAR contains invalid file name '..'" + +# Likewise for a filename containing a slash. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < slash.nar | grepQuiet "NAR contains invalid file name 'x/y'" + +# Likewise for an empty filename. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < empty.nar | grepQuiet "NAR contains invalid file name ''" + +# Test that the 'executable' field cannot come before the 'contents' field. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < executable-after-contents.nar | grepQuiet "expected tag ')', got 'executable'" + +# Test that the 'name' field cannot come before the 'node' field in a directory entry. +rm -rf "$TEST_ROOT/out" +expectStderr 1 nix-store --restore "$TEST_ROOT/out" < name-after-node.nar | grepQuiet "expected tag 'name'" diff --git a/tests/functional/nul.nar b/tests/functional/nul.nar new file mode 100644 index 000000000..9ae48baf6 Binary files /dev/null and b/tests/functional/nul.nar differ diff --git a/tests/functional/slash.nar b/tests/functional/slash.nar new file mode 100644 index 000000000..118a60216 Binary files /dev/null and b/tests/functional/slash.nar differ