about summary refs log tree commit diff
path: root/src/nix-store/nix-store.cc
diff options
context:
space:
mode:
authorEelco Dolstra <eelco.dolstra@logicblox.com>2013-02-26T01·30+0100
committerEelco Dolstra <eelco.dolstra@logicblox.com>2013-02-26T01·30+0100
commit5526a282b5b44e9296e61e07d7d2626a79141ac4 (patch)
tree47d50327dc39bc716fa9ef047ab2b6d2667340e0 /src/nix-store/nix-store.cc
parentdadf7a5b46f08b59c7e15a40937a9039ef273d63 (diff)
Security: Don't allow builders to change permissions on files they don't own
It turns out that in multi-user Nix, a builder may be able to do

  ln /etc/shadow $out/foo

Afterwards, canonicalisePathMetaData() will be applied to $out/foo,
causing /etc/shadow's mode to be set to 444 (readable by everybody but
writable by nobody).  That's obviously Very Bad.

Fortunately, this fails in NixOS's default configuration because
/nix/store is a bind mount, so "ln" will fail with "Invalid
cross-device link".  It also fails if hard-link restrictions are
enabled, so a workaround is:

  echo 1 > /proc/sys/fs/protected_hardlinks

The solution is to check that all files in $out are owned by the build
user.  This means that innocuous operations like "ln
${pkgs.foo}/some-file $out/" are now rejected, but that already failed
in chroot builds anyway.
Diffstat (limited to 'src/nix-store/nix-store.cc')
-rw-r--r--src/nix-store/nix-store.cc10
1 files changed, 2 insertions, 8 deletions
diff --git a/src/nix-store/nix-store.cc b/src/nix-store/nix-store.cc
index e41922304031..151ed97e4b8b 100644
--- a/src/nix-store/nix-store.cc
+++ b/src/nix-store/nix-store.cc
@@ -238,12 +238,6 @@ static void printTree(const Path & path,
     PathSet references;
     store->queryReferences(path, references);
 
-#if 0
-    for (PathSet::iterator i = drv.inputSrcs.begin();
-         i != drv.inputSrcs.end(); ++i)
-        cout << format("%1%%2%\n") % (tailPad + treeConn) % *i;
-#endif
-
     /* Topologically sort under the relation A < B iff A \in
        closure(B).  That is, if derivation A is an (possibly indirect)
        input of B, then A is printed first.  This has the effect of
@@ -251,7 +245,7 @@ static void printTree(const Path & path,
     Paths sorted = topoSortPaths(*store, references);
     reverse(sorted.begin(), sorted.end());
 
-    for (Paths::iterator i = sorted.begin(); i != sorted.end(); ++i) {
+    foreach (Paths::iterator, i, sorted) {
         Paths::iterator j = i; ++j;
         printTree(*i, tailPad + treeConn,
             j == sorted.end() ? tailPad + treeNull : tailPad + treeLine,
@@ -521,7 +515,7 @@ static void registerValidity(bool reregister, bool hashGiven, bool canonicalise)
         if (!store->isValidPath(info.path) || reregister) {
             /* !!! races */
             if (canonicalise)
-                canonicalisePathMetaData(info.path);
+                canonicalisePathMetaData(info.path, -1);
             if (!hashGiven) {
                 HashResult hash = hashPath(htSHA256, info.path);
                 info.hash = hash.first;