diff options
Diffstat (limited to 'src/libutil')
| -rw-r--r-- | src/libutil/archive.cc | 156 | ||||
| -rw-r--r-- | src/libutil/archive.hh | 72 | ||||
| -rw-r--r-- | src/libutil/file-system.cc | 14 | ||||
| -rw-r--r-- | src/libutil/file-system.hh | 9 |
4 files changed, 156 insertions, 95 deletions
diff --git a/src/libutil/archive.cc b/src/libutil/archive.cc index d4da18f14..225483804 100644 --- a/src/libutil/archive.cc +++ b/src/libutil/archive.cc @@ -334,7 +334,7 @@ Generator<Entry> parse(Source & source) } -static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar) +static WireFormatGenerator restore(NARParseVisitor & sink, Generator<nar::Entry> nar) { while (auto entry = nar.next()) { co_yield std::visit( @@ -347,16 +347,13 @@ static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar) }, [&](nar::File f) { return [](auto f, auto & sink) -> WireFormatGenerator { - sink.createRegularFile(f.path); - sink.preallocateContents(f.size); - if (f.executable) { - sink.isExecutable(); - } + auto handle = sink.createRegularFile(f.path, f.size, f.executable); + while (auto block = f.contents.next()) { - sink.receiveContents(std::string_view{block->data(), block->size()}); + handle->receiveContents(std::string_view{block->data(), block->size()}); co_yield *block; } - sink.closeRegularFile(); + handle->close(); }(std::move(f), sink); }, [&](nar::Symlink sl) { @@ -377,12 +374,12 @@ static WireFormatGenerator restore(ParseSink & sink, Generator<nar::Entry> nar) } } -WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source) +WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source) { return restore(sink, nar::parse(source)); } -void parseDump(ParseSink & sink, Source & source) +void parseDump(NARParseVisitor & sink, Source & source) { auto parser = parseAndCopyDump(sink, source); while (parser.next()) { @@ -390,11 +387,99 @@ void parseDump(ParseSink & sink, Source & source) } } -struct RestoreSink : ParseSink +/* + * Note [NAR restoration security]: + * It's *critical* that NAR restoration will never overwrite anything even if + * duplicate filenames are passed in. It is inevitable that not all NARs are + * fit to actually successfully restore to the target filesystem; errors may + * occur due to collisions, and this *must* cause the NAR to be rejected. + * + * Although the filenames are blocked from being *the same bytes* by a higher + * layer, filesystems have other ideas on every platform: + * - The store may be on a case-insensitive filesystem like APFS, ext4 with + * casefold directories, zfs with casesensitivity=insensitive + * - The store may be on a Unicode normalizing (or normalization-insensitive) + * filesystem like APFS (where files are looked up by + * hash(normalize(fname))), HFS+ (where file names are always normalized to + * approximately NFD), or zfs with normalization=formC, etc. + * + * It is impossible to know the version of Unicode being used by the underlying + * filesystem, thus it is *impossible* to stop these collisions. + * + * Overwriting files as a result of invalid NARs will cause a security bug like + * CppNix's CVE-2024-45593 (GHSA-h4vv-h3jq-v493) + */ + +/** + * This code restores NARs from disk. + * + * See Note [NAR restoration security] for security invariants in this procedure. + * + */ +struct NARRestoreVisitor : NARParseVisitor { Path dstPath; - AutoCloseFD fd; +private: + class MyFileHandle : public FileHandle + { + AutoCloseFD fd; + + MyFileHandle(AutoCloseFD && fd, uint64_t size, bool executable) : FileHandle(), fd(std::move(fd)) + { + if (executable) { + makeExecutable(); + } + + maybePreallocateContents(size); + } + + void makeExecutable() + { + struct stat st; + if (fstat(fd.get(), &st) == -1) + throw SysError("fstat"); + if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1) + throw SysError("fchmod"); + } + + void maybePreallocateContents(uint64_t len) + { + if (!archiveSettings.preallocateContents) + return; + +#if HAVE_POSIX_FALLOCATE + if (len) { + errno = posix_fallocate(fd.get(), 0, len); + /* Note that EINVAL may indicate that the underlying + filesystem doesn't support preallocation (e.g. on + OpenSolaris). Since preallocation is just an + optimisation, ignore it. */ + if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS) + throw SysError("preallocating file of %1% bytes", len); + } +#endif + } + + public: + + ~MyFileHandle() = default; + + virtual void close() override + { + /* Call close explicitly to make sure the error is checked */ + fd.close(); + } + + void receiveContents(std::string_view data) override + { + writeFull(fd.get(), data); + } + + friend struct NARRestoreVisitor; + }; + +public: void createDirectory(const Path & path) override { Path p = dstPath + path; @@ -402,49 +487,13 @@ struct RestoreSink : ParseSink throw SysError("creating directory '%1%'", p); }; - void createRegularFile(const Path & path) override + std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) override { Path p = dstPath + path; - fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)}; + AutoCloseFD fd = AutoCloseFD{open(p.c_str(), O_CREAT | O_EXCL | O_WRONLY | O_CLOEXEC, 0666)}; if (!fd) throw SysError("creating file '%1%'", p); - } - - void closeRegularFile() override - { - /* Call close explicitly to make sure the error is checked */ - fd.close(); - } - - void isExecutable() override - { - struct stat st; - if (fstat(fd.get(), &st) == -1) - throw SysError("fstat"); - if (fchmod(fd.get(), st.st_mode | (S_IXUSR | S_IXGRP | S_IXOTH)) == -1) - throw SysError("fchmod"); - } - - void preallocateContents(uint64_t len) override - { - if (!archiveSettings.preallocateContents) - return; - -#if HAVE_POSIX_FALLOCATE - if (len) { - errno = posix_fallocate(fd.get(), 0, len); - /* Note that EINVAL may indicate that the underlying - filesystem doesn't support preallocation (e.g. on - OpenSolaris). Since preallocation is just an - optimisation, ignore it. */ - if (errno && errno != EINVAL && errno != EOPNOTSUPP && errno != ENOSYS) - throw SysError("preallocating file of %1% bytes", len); - } -#endif - } - void receiveContents(std::string_view data) override - { - writeFull(fd.get(), data); + return std::unique_ptr<MyFileHandle>(new MyFileHandle(std::move(fd), size, executable)); } void createSymlink(const Path & path, const std::string & target) override @@ -457,7 +506,7 @@ struct RestoreSink : ParseSink void restorePath(const Path & path, Source & source) { - RestoreSink sink; + NARRestoreVisitor sink; sink.dstPath = path; parseDump(sink, source); } @@ -468,10 +517,9 @@ WireFormatGenerator copyNAR(Source & source) // FIXME: if 'source' is the output of dumpPath() followed by EOF, // we should just forward all data directly without parsing. - static ParseSink parseSink; /* null sink; just parse the NAR */ + static NARParseVisitor parseSink; /* null sink; just parse the NAR */ return parseAndCopyDump(parseSink, source); } - } diff --git a/src/libutil/archive.hh b/src/libutil/archive.hh index b34d06e3d..c633bee00 100644 --- a/src/libutil/archive.hh +++ b/src/libutil/archive.hh @@ -76,45 +76,47 @@ WireFormatGenerator dumpString(std::string_view s); /** * \todo Fix this API, it sucks. + * A visitor for NAR parsing that performs filesystem (or virtual-filesystem) + * actions to restore a NAR. + * + * Methods of this may arbitrarily fail due to filename collisions. */ -struct ParseSink -{ - virtual void createDirectory(const Path & path) { }; - - virtual void createRegularFile(const Path & path) { }; - virtual void closeRegularFile() { }; - virtual void isExecutable() { }; - virtual void preallocateContents(uint64_t size) { }; - virtual void receiveContents(std::string_view data) { }; - - virtual void createSymlink(const Path & path, const std::string & target) { }; -}; - -/** - * If the NAR archive contains a single file at top-level, then save - * the contents of the file to `s`. Otherwise barf. - */ -struct RetrieveRegularNARSink : ParseSink +struct NARParseVisitor { - bool regular = true; - Sink & sink; - - RetrieveRegularNARSink(Sink & sink) : sink(sink) { } - - void createDirectory(const Path & path) override + /** + * A type-erased file handle specific to this particular NARParseVisitor. + */ + struct FileHandle { - regular = false; - } - - void receiveContents(std::string_view data) override + FileHandle() {} + FileHandle(FileHandle const &) = delete; + FileHandle & operator=(FileHandle &) = delete; + + /** Puts one block of data into the file */ + virtual void receiveContents(std::string_view data) { } + + /** + * Explicitly closes the file. Further operations may throw an assert. + * This exists so that closing can fail and throw an exception without doing so in a destructor. + */ + virtual void close() { } + + virtual ~FileHandle() = default; + }; + + virtual void createDirectory(const Path & path) { } + + /** + * Creates a regular file in the extraction output with the given size and executable flag. + * The size is guaranteed to be the true size of the file. + */ + [[nodiscard]] + virtual std::unique_ptr<FileHandle> createRegularFile(const Path & path, uint64_t size, bool executable) { - sink(data); + return std::make_unique<FileHandle>(); } - void createSymlink(const Path & path, const std::string & target) override - { - regular = false; - } + virtual void createSymlink(const Path & path, const std::string & target) { } }; namespace nar { @@ -160,8 +162,8 @@ Generator<Entry> parse(Source & source); } -WireFormatGenerator parseAndCopyDump(ParseSink & sink, Source & source); -void parseDump(ParseSink & sink, Source & source); +WireFormatGenerator parseAndCopyDump(NARParseVisitor & sink, Source & source); +void parseDump(NARParseVisitor & sink, Source & source); void restorePath(const Path & path, Source & source); diff --git a/src/libutil/file-system.cc b/src/libutil/file-system.cc index 8c69c9864..1d3eba58f 100644 --- a/src/libutil/file-system.cc +++ b/src/libutil/file-system.cc @@ -18,15 +18,19 @@ namespace fs = std::filesystem; namespace nix { +Path getCwd() { + char buf[PATH_MAX]; + if (!getcwd(buf, sizeof(buf))) { + throw SysError("cannot get cwd"); + } + return Path(buf); +} + Path absPath(Path path, std::optional<PathView> dir, bool resolveSymlinks) { if (path.empty() || path[0] != '/') { if (!dir) { - char buf[PATH_MAX]; - if (!getcwd(buf, sizeof(buf))) { - throw SysError("cannot get cwd"); - } - path = concatStrings(buf, "/", path); + path = concatStrings(getCwd(), "/", path); } else { path = concatStrings(*dir, "/", path); } diff --git a/src/libutil/file-system.hh b/src/libutil/file-system.hh index 0a54d1a3b..2547c63b5 100644 --- a/src/libutil/file-system.hh +++ b/src/libutil/file-system.hh @@ -30,6 +30,13 @@ struct Sink; struct Source; /** + * Get the current working directory. + * + * Throw an error if the current directory cannot get got. + */ +Path getCwd(); + +/** * @return An absolutized path, resolving paths relative to the * specified directory, or the current directory otherwise. The path * is also canonicalised. @@ -203,7 +210,7 @@ inline Paths createDirs(PathView path) } /** - * Create a symlink. + * Create a symlink. Throws if the symlink exists. */ void createSymlink(const Path & target, const Path & link); |