* Fix a huuuuge security hole in the Nix daemon. It didn't check that

  derivations added to the store by clients have "correct" output
  paths (meaning that the output paths are computed by hashing the
  derivation according to a certain algorithm).  This means that a
  malicious user could craft a special .drv file to build *any*
  desired path in the store with any desired contents (so long as the
  path doesn't already exist).  Then the attacker just needs to wait
  for a victim to come along and install the compromised path.

  For instance, if Alice (the attacker) knows that the latest Firefox
  derivation in Nixpkgs produces the path

    /nix/store/1a5nyfd4ajxbyy97r1fslhgrv70gj8a7-firefox-5.0.1

  then (provided this path doesn't already exist) she can craft a .drv
  file that creates that path (i.e., has it as one of its outputs),
  add it to the store using "nix-store --add", and build it with
  "nix-store -r".  So the fake .drv could write a Trojan to the
  Firefox path.  Then, if user Bob (the victim) comes along and does

    $ nix-env -i firefox
    $ firefox

  he executes the Trojan injected by Alice.

  The fix is to have the Nix daemon verify that derivation outputs are
  correct (in addValidPath()).  This required some refactoring to move
  the hash computation code to libstore.

1 files changed, 51 insertions, 0 deletions

diff --git a/src/libstore/local-store.cc b/src/libstore/local-store.cc
index 6af34cc778..691069e2b8 100644
--- a/src/libstore/local-store.cc
+++ b/src/libstore/local-store.cc
@@ -469,6 +469,47 @@ void canonicalisePathMetaData(const Path & path)
 }
 
 
+void LocalStore::checkDerivationOutputs(const Path & drvPath, const Derivation & drv)
+{
+    string drvName = storePathToName(drvPath);
+    assert(isDerivation(drvName));
+    drvName = string(drvName, 0, drvName.size() - drvExtension.size());
+        
+    if (isFixedOutputDrv(drv)) {
+        DerivationOutputs::const_iterator out = drv.outputs.find("out");
+        if (out == drv.outputs.end())
+            throw Error(format("derivation `%1%' does not have an output named `out'") % drvPath);
+
+        bool recursive; HashType ht; Hash h;
+        out->second.parseHashInfo(recursive, ht, h);
+        Path outPath = makeFixedOutputPath(recursive, ht, h, drvName);
+
+        StringPairs::const_iterator j = drv.env.find("out");
+        if (out->second.path != outPath || j == drv.env.end() || j->second != outPath)
+            throw Error(format("derivation `%1%' has incorrect output `%2%', should be `%3%'")
+                % drvPath % out->second.path % outPath);
+    }
+
+    else {
+        Derivation drvCopy(drv);
+        foreach (DerivationOutputs::iterator, i, drvCopy.outputs) {
+            i->second.path = "";
+            drvCopy.env[i->first] = "";
+        }
+
+        Hash h = hashDerivationModulo(drvCopy);
+        
+        foreach (DerivationOutputs::const_iterator, i, drv.outputs) {
+            Path outPath = makeOutputPath(i->first, h, drvName);
+            StringPairs::const_iterator j = drv.env.find(i->first);
+            if (i->second.path != outPath || j == drv.env.end() || j->second != outPath)
+                throw Error(format("derivation `%1%' has incorrect output `%2%', should be `%3%'")
+                    % drvPath % i->second.path % outPath);
+        }
+    }
+}
+
+
 unsigned long long LocalStore::addValidPath(const ValidPathInfo & info)
 {
     SQLiteStmtUse use(stmtRegisterValidPath);
@@ -493,6 +534,14 @@ unsigned long long LocalStore::addValidPath(const ValidPathInfo & info)
        derivation. */
     if (isDerivation(info.path)) {
         Derivation drv = parseDerivation(readFile(info.path));
+        
+        /* Verify that the output paths in the derivation are correct
+           (i.e., follow the scheme for computing output paths from
+           derivations).  Note that if this throws an error, then the
+           DB transaction is rolled back, so the path validity
+           registration above is undone. */
+        checkDerivationOutputs(info.path, drv);
+        
         foreach (DerivationOutputs::iterator, i, drv.outputs) {
             SQLiteStmtUse use(stmtAddDerivationOutput);
             stmtAddDerivationOutput.bind(id);
@@ -925,6 +974,8 @@ void LocalStore::invalidatePath(const Path & path)
 {
     debug(format("invalidating path `%1%'") % path);
 
+    drvHashes.erase(path);
+
     SQLiteStmtUse use(stmtInvalidatePath);
 
     stmtInvalidatePath.bind(path);