From 021dab162f7cce726700ec783db90f2e161aff36 Mon Sep 17 00:00:00 2001 From: Robert Hensing Date: Fri, 13 Dec 2024 17:21:56 +0100 Subject: [PATCH] Fix `nix hash` symlink and fifo behavior Fixes https://github.com/NixOS/nix/issues/11756 Fixes https://github.com/NixOS/nix/issues/11681 --- src/libutil/posix-source-accessor.hh | 14 +++++++++++++- src/nix/hash.cc | 23 ++++++++++++++++++++--- tests/functional/hash-path.sh | 24 ++++++++++++++++++++++++ 3 files changed, 57 insertions(+), 4 deletions(-) diff --git a/src/libutil/posix-source-accessor.hh b/src/libutil/posix-source-accessor.hh index 40f60bb54b8..b87a81929c8 100644 --- a/src/libutil/posix-source-accessor.hh +++ b/src/libutil/posix-source-accessor.hh @@ -43,13 +43,25 @@ struct PosixSourceAccessor : virtual SourceAccessor std::optional getPhysicalPath(const CanonPath & path) override; /** - * Create a `PosixSourceAccessor` and `CanonPath` corresponding to + * Create a `PosixSourceAccessor` and `SourcePath` corresponding to * some native path. * * The `PosixSourceAccessor` is rooted as far up the tree as * possible, (e.g. on Windows it could scoped to a drive like * `C:\`). This allows more `..` parent accessing to work. * + * @note When `path` is trusted user input, canonicalize it using + * `std::filesystem::canonical`, `fs::parent_canonical`, `std::filesystem::weakly_canonical`, etc, + * as appropriate for the use case. At least weak canonicalization is + * required for the `SourcePath` to do anything useful at the location it + * points to. + * + * @note A canonicalizing behavior is not built in `createAtRoot` so that + * callers do not accidentally introduce symlink-related security vulnerabilities. + * Furthermore, `createAtRoot` does not know whether the file pointed to by + * `path` should be resolved if it is itself a symlink. In other words, + * `createAtRoot` can not decide between aforemention `canonical`, `parent_canonical`, etc. for its callers. + * * See * [`std::filesystem::path::root_path`](https://en.cppreference.com/w/cpp/filesystem/path/root_path) * and diff --git a/src/nix/hash.cc b/src/nix/hash.cc index 416cd19b317..33060a4be9b 100644 --- a/src/nix/hash.cc +++ b/src/nix/hash.cc @@ -87,18 +87,35 @@ struct CmdHashBase : Command return std::make_unique(hashAlgo); }; - auto path2 = PosixSourceAccessor::createAtRoot(path); + auto makeSourcePath = [&]() -> SourcePath { + return PosixSourceAccessor::createAtRoot(std::filesystem::weakly_canonical(path)); + }; + Hash h { HashAlgorithm::SHA256 }; // throwaway def to appease C++ switch (mode) { case FileIngestionMethod::Flat: + { + // While usually we could use the some code as for NixArchive, + // the Flat method needs to support FIFOs, such as those + // produced by bash process substitution, e.g.: + // nix hash --mode flat <(echo hi) + // Also symlinks semantics are unambiguous in the flat case, + // so we don't need to go low-level, or reject symlink `path`s. + auto hashSink = makeSink(); + readFile(path, *hashSink); + h = hashSink->finish().first; + break; + } case FileIngestionMethod::NixArchive: { + auto sourcePath = makeSourcePath(); auto hashSink = makeSink(); - dumpPath(path2, *hashSink, (FileSerialisationMethod) mode); + dumpPath(sourcePath, *hashSink, (FileSerialisationMethod) mode); h = hashSink->finish().first; break; } case FileIngestionMethod::Git: { + auto sourcePath = PosixSourceAccessor::createAtRoot(path); std::function hook; hook = [&](const SourcePath & path) -> git::TreeEntry { auto hashSink = makeSink(); @@ -109,7 +126,7 @@ struct CmdHashBase : Command .hash = hash, }; }; - h = hook(path2).hash; + h = hook(sourcePath).hash; break; } } diff --git a/tests/functional/hash-path.sh b/tests/functional/hash-path.sh index 86d782a9589..74e3195045a 100755 --- a/tests/functional/hash-path.sh +++ b/tests/functional/hash-path.sh @@ -92,3 +92,27 @@ try2 md5 "20f3ffe011d4cfa7d72bfabef7882836" rm "$TEST_ROOT/hash-path/hello" ln -s x "$TEST_ROOT/hash-path/hello" try2 md5 "f78b733a68f5edbdf9413899339eaa4a" + +# Flat mode supports process substitution +h=$(nix hash path --mode flat --type sha256 --base32 <(printf "SMASH THE STATE")) +[[ 0d9n3r2i4m1zgy0wpqbsyabsfzgs952066bfp8gwvcg4mkr4r5g8 == "$h" ]] + +# Flat mode supports process substitution (hash file) +h=$(nix hash file --type sha256 --base32 <(printf "SMASH THE STATE")) +[[ 0d9n3r2i4m1zgy0wpqbsyabsfzgs952066bfp8gwvcg4mkr4r5g8 == "$h" ]] + +# Symlinks in the ancestry are ok and don't affect the result +mkdir -p "$TEST_ROOT/simple" "$TEST_ROOT/try/to/mess/with/it" +echo hi > "$TEST_ROOT/simple/hi" +ln -s "$TEST_ROOT/simple" "$TEST_ROOT/try/to/mess/with/it/simple-link" +h=$(nix hash path --type sha256 --base32 "$TEST_ROOT/simple/hi") +[[ 1xmr8jicvzszfzpz46g37mlpvbzjl2wpwvl2b05psipssyp1sm8h == "$h" ]] +h=$(nix hash path --type sha256 --base32 "$TEST_ROOT/try/to/mess/with/it/simple-link/hi") +[[ 1xmr8jicvzszfzpz46g37mlpvbzjl2wpwvl2b05psipssyp1sm8h == "$h" ]] + +# nix hash --mode nar does not canonicalize a symlink argument. +# Otherwise it can't generate a NAR whose root is a symlink. +# If you want to follow the symlink, pass $(realpath -s ...) instead. +ln -s /non-existent-48cujwe8ndf4as0bne "$TEST_ROOT/symlink-to-nowhere" +h=$(nix hash path --mode nar --type sha256 --base32 "$TEST_ROOT/symlink-to-nowhere") +[[ 1bl5ry3x1fcbwgr5c2x50bn572iixh4j1p6ax5isxly2ddgn8pbp == "$h" ]] # manually verified hash