Merge commit from fork

Fix unsafe NAR unpacking

src/libutil/archive.cc

@@ -23,7 +23,7 @@ struct ArchiveSettings : Config
             false,
         #endif
         "use-case-hack",
-        "Whether to enable a Darwin-specific hack for dealing with file name collisions."};
+        "Whether to enable a macOS-specific hack for dealing with file name case collisions."};
 };
 
 static ArchiveSettings archiveSettings;
@@ -214,11 +214,13 @@ static void parse(FileSystemObjectSink & sink, Source & source, const CanonPath
             else if (t == "directory") {
                 sink.createDirectory(path);
 
+                std::string prevName;
+
                 while (1) {
                     s = getString();
 
                     if (s == "entry") {
-                        std::string name, prevName;
+                        std::string name;
 
                         s = getString();
                         if (s != "(") throw badArchive("expected open tag");
@@ -241,6 +243,9 @@ static void parse(FileSystemObjectSink & sink, Source & source, const CanonPath
                                         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;
                                 }

src/libutil/fs-sink.cc

@@ -68,10 +68,19 @@ static RestoreSinkSettings restoreSinkSettings;
 
 static GlobalConfig::Register r1(&restoreSinkSettings);
 
+static std::filesystem::path append(const std::filesystem::path & src, const CanonPath & path)
+{
+    auto dst = src;
+    if (!path.rel().empty())
+        dst /= path.rel();
+    return dst;
+}
 
 void RestoreSink::createDirectory(const CanonPath & path)
 {
-    std::filesystem::create_directory(dstPath / path.rel());
+    auto p = append(dstPath, path);
+    if (!std::filesystem::create_directory(p))
+        throw Error("path '%s' already exists", p.string());
 };
 
 struct RestoreRegularFile : CreateRegularFileSink {
@@ -93,14 +102,6 @@ struct RestoreRegularFile : CreateRegularFileSink {
     void preallocateContents(uint64_t size) override;
 };
 
-static std::filesystem::path append(const std::filesystem::path & src, const CanonPath & path)
-{
-    auto dst = src;
-    if (!path.rel().empty())
-        dst /= path.rel();
-    return dst;
-}
-
 void RestoreSink::createRegularFile(const CanonPath & path, std::function<void(CreateRegularFileSink &)> func)
 {
     auto p = append(dstPath, path);

tests/functional/case-hack.sh

@@ -1,24 +0,0 @@
-#!/usr/bin/env bash
-
-source common.sh
-
-TODO_NixOS
-
-clearStore
-
-rm -rf "$TEST_ROOT/case"
-
-opts=("--option" "use-case-hack" "true")
-
-# Check whether restoring and dumping a NAR that contains case
-# collisions is round-tripping, even on a case-insensitive system.
-
-nix-store "${opts[@]}" --restore "$TEST_ROOT/case" < case.nar
-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)" ]
-
-# Check whether we detect true collisions (e.g. those remaining after
-# removal of the suffix).
-touch "$TEST_ROOT/case/xt_CONNMARK.h~nix~case~hack~3"
-(! nix-store "${opts[@]}" --dump "$TEST_ROOT/case" > /dev/null)

tests/functional/local.mk

@@ -90,7 +90,7 @@ nix_tests = \
   derivation-advanced-attributes.sh \
   import-derivation.sh \
   nix_path.sh \
-  case-hack.sh \
+  nars.sh \
   placeholders.sh \
   ssh-relay.sh \
   build.sh \

tests/functional/meson.build

@@ -159,7 +159,7 @@ suites = [
       'derivation-advanced-attributes.sh',
       'import-derivation.sh',
       'nix_path.sh',
-      'case-hack.sh',
+      'nars.sh',
       'placeholders.sh',
       'ssh-relay.sh',
       'build.sh',

tests/functional/nars.sh

@@ -0,0 +1,94 @@
+#!/usr/bin/env bash
+
+source common.sh
+
+TODO_NixOS
+
+clearStore
+
+# Check that NARs with duplicate directory entries are rejected.
+rm -rf "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < duplicate.nar | grepQuiet "NAR directory is not sorted"
+
+# Check that nix-store --restore fails if the output already exists.
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < duplicate.nar | grepQuiet "path '.*/out' already exists"
+
+rm -rf "$TEST_ROOT/out"
+echo foo > "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < duplicate.nar | grepQuiet "File exists"
+
+rm -rf "$TEST_ROOT/out"
+ln -s "$TEST_ROOT/out2" "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < duplicate.nar | grepQuiet "File exists"
+
+mkdir -p "$TEST_ROOT/out2"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < duplicate.nar | grepQuiet "path '.*/out' already exists"
+
+# The same, but for a regular file.
+nix-store --dump ./nars.sh > "$TEST_ROOT/tmp.nar"
+
+rm -rf "$TEST_ROOT/out"
+nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+rm -rf "$TEST_ROOT/out"
+mkdir -p "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+rm -rf "$TEST_ROOT/out"
+ln -s "$TEST_ROOT/out2" "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+mkdir -p "$TEST_ROOT/out2"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+# The same, but for a symlink.
+ln -sfn foo "$TEST_ROOT/symlink"
+nix-store --dump "$TEST_ROOT/symlink" > "$TEST_ROOT/tmp.nar"
+
+rm -rf "$TEST_ROOT/out"
+nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar"
+[[ -L "$TEST_ROOT/out" ]]
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+rm -rf "$TEST_ROOT/out"
+mkdir -p "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+rm -rf "$TEST_ROOT/out"
+ln -s "$TEST_ROOT/out2" "$TEST_ROOT/out"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+mkdir -p "$TEST_ROOT/out2"
+expectStderr 1 nix-store --restore "$TEST_ROOT/out" < "$TEST_ROOT/tmp.nar" | grepQuiet "File exists"
+
+# Check whether restoring and dumping a NAR that contains case
+# collisions is round-tripping, even on a case-insensitive system.
+rm -rf "$TEST_ROOT/case"
+opts=("--option" "use-case-hack" "true")
+nix-store "${opts[@]}" --restore "$TEST_ROOT/case" < case.nar
+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)" ]
+
+# Check whether we detect true collisions (e.g. those remaining after
+# removal of the suffix).
+touch "$TEST_ROOT/case/xt_CONNMARK.h~nix~case~hack~3"
+(! nix-store "${opts[@]}" --dump "$TEST_ROOT/case" > /dev/null)
+
+# Detect NARs that have a directory entry that after case-hacking
+# collides with another entry (e.g. a directory containing 'Test',
+# 'Test~nix~case~hack~1' and 'test').
+rm -rf "$TEST_ROOT/case"
+expectStderr 1 nix-store "${opts[@]}" --restore "$TEST_ROOT/case" < case-collision.nar | grepQuiet "NAR contains file name 'test' that collides with case-hacked file name 'Test~nix~case~hack~1'"
+
+# Deserializing a NAR that contains file names that Unicode-normalize
+# to the same name should fail on macOS but succeed on Linux.
+rm -rf "$TEST_ROOT/out"
+if [[ $(uname) = Darwin ]]; then
+    expectStderr 1 nix-store --restore "$TEST_ROOT/out" < unnormalized.nar | grepQuiet "path '.*/out/â' already exists"
+else
+    nix-store --restore "$TEST_ROOT/out" < unnormalized.nar
+    [[ -e $TEST_ROOT/out/â ]]
+    [[ -e $TEST_ROOT/out/â ]]
+fi