01/05: Revert "daemon: Fix possible use-after-free."

[Ludovic Courtès]

civodul pushed a commit to branch master
in repository guix.

commit a88b8c5c985a87586159c0621974a1dfe5b9b92d
Author: Ludovic Courtès <address@hidden>
Date:   Tue May 19 08:02:52 2015 +0200

    Revert "daemon: Fix possible use-after-free."
    
    This reverts commit 1303a4a4517260def862ce7fe97e6b28dd8005e1.
---
 nix/libstore/build.cc |   29 ++++++++++++++++++++++-------
 nix/libutil/util.cc   |   20 ++++++--------------
 nix/libutil/util.hh   |    5 -----
 3 files changed, 28 insertions(+), 26 deletions(-)

diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc
index b3c994d..f38cd29 100644
--- a/nix/libstore/build.cc
+++ b/nix/libstore/build.cc
@@ -401,6 +401,18 @@ static void commonChildInit(Pipe & logPipe)
 }
 
 
+/* Convert a string list to an array of char pointers.  Careful: the
+   string list should outlive the array. */
+const char * * strings2CharPtrs(const Strings & ss)
+{
+    const char * * arr = new const char * [ss.size() + 1];
+    const char * * p = arr;
+    foreach (Strings::const_iterator, i, ss) *p++ = i->c_str();
+    *p = 0;
+    return arr;
+}
+
+
 /* Restore default handling of SIGPIPE, otherwise some programs will
    randomly say "Broken pipe". */
 static void restoreSIGPIPE()
@@ -2123,7 +2135,11 @@ void DerivationGoal::initChild()
         Strings envStrs;
         foreach (Environment::const_iterator, i, env)
             envStrs.push_back(rewriteHashes(i->first + "=" + i->second, 
rewritesToTmp));
-       std::vector<const char *> envArr = stringsToCharPtrs(envStrs);
+        const char * * envArr = strings2CharPtrs(envStrs);
+
+        Path program = drv.builder.c_str();
+        std::vector<const char *> args; /* careful with c_str()! */
+        string user; /* must be here for its c_str()! */
 
         /* If we are running in `build-users' mode, then switch to the
            user we allocated above.  Make sure that we drop all root
@@ -2149,18 +2165,17 @@ void DerivationGoal::initChild()
         }
 
         /* Fill in the arguments. */
-       Strings args;
         string builderBasename = baseNameOf(drv.builder);
         args.push_back(builderBasename.c_str());
         foreach (Strings::iterator, i, drv.args)
-            args.push_back(rewriteHashes(*i, rewritesToTmp));
-       std::vector<const char *> argArr = stringsToCharPtrs(args);
+            args.push_back(rewriteHashes(*i, rewritesToTmp).c_str());
+        args.push_back(0);
 
         restoreSIGPIPE();
 
         /* Execute the program.  This should not return. */
         inSetup = false;
-        execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) 
&envArr[0]);
+        execve(program.c_str(), (char * *) &args[0], (char * *) envArr);
 
         throw SysError(format("executing `%1%'") % drv.builder);
 
@@ -2763,7 +2778,7 @@ void SubstitutionGoal::tryToRun()
     args.push_back("--substitute");
     args.push_back(storePath);
     args.push_back(destPath);
-    std::vector<const char *> argArr = stringsToCharPtrs(args);
+    const char * * argArr = strings2CharPtrs(args);
 
     /* Fork the substitute program. */
     pid = maybeVfork();
@@ -2781,7 +2796,7 @@ void SubstitutionGoal::tryToRun()
             if (dup2(outPipe.writeSide, STDOUT_FILENO) == -1)
                 throw SysError("cannot dup output pipe into stdout");
 
-            execv(sub.c_str(), (char * *) &argArr[0]);
+            execv(sub.c_str(), (char * *) argArr);
 
             throw SysError(format("executing `%1%'") % sub);
 
diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc
index 024cea8..846674a 100644
--- a/nix/libutil/util.cc
+++ b/nix/libutil/util.cc
@@ -852,20 +852,16 @@ void killUser(uid_t uid)
 //////////////////////////////////////////////////////////////////////
 
 
-std::vector<const char *> stringsToCharPtrs(const Strings & ss)
-{
-    std::vector<const char *> res;
-    foreach (Strings::const_iterator, i, ss)
-        res.push_back(i->c_str());
-    res.push_back(0);
-    return res;
-}
-
-
 string runProgram(Path program, bool searchPath, const Strings & args)
 {
     checkInterrupt();
 
+    std::vector<const char *> cargs; /* careful with c_str()! */
+    cargs.push_back(program.c_str());
+    for (Strings::const_iterator i = args.begin(); i != args.end(); ++i)
+        cargs.push_back(i->c_str());
+    cargs.push_back(0);
+
     /* Create a pipe. */
     Pipe pipe;
     pipe.create();
@@ -884,10 +880,6 @@ string runProgram(Path program, bool searchPath, const 
Strings & args)
             if (dup2(pipe.writeSide, STDOUT_FILENO) == -1)
                 throw SysError("dupping stdout");
 
-           Strings args_(args);
-           args_.push_front(program);
-           auto cargs = stringsToCharPtrs(args_);
-
             if (searchPath)
                 execvp(program.c_str(), (char * *) &cargs[0]);
             else
diff --git a/nix/libutil/util.hh b/nix/libutil/util.hh
index a709818..ce2d77c 100644
--- a/nix/libutil/util.hh
+++ b/nix/libutil/util.hh
@@ -257,11 +257,6 @@ void killUser(uid_t uid);
 string runProgram(Path program, bool searchPath = false,
     const Strings & args = Strings());
 
-/* Convert a list of strings to a null-terminated vector of char
-   *'s. The result must not be accessed beyond the lifetime of the
-   list of strings. */
-std::vector<const char *> stringsToCharPtrs(const Strings & ss);
-
 /* Close all file descriptors except stdin, stdout, stderr, and those
    listed in the given set.  Good practice in child processes. */
 void closeMostFDs(const set<int> & exceptions);