From 1303a4a4517260def862ce7fe97e6b28dd8005e1 Mon Sep 17 00:00:00 2001 From: Ludovic Courtès Date: Mon, 11 May 2015 22:21:31 +0200 Subject: daemon: Fix possible use-after-free. This is essentially a backport of by Eelco Dolstra . The use-after-free bug would typically manifest when building with GCC 5.1. --- nix/libstore/build.cc | 29 +++++++---------------------- nix/libutil/util.cc | 20 ++++++++++++++------ nix/libutil/util.hh | 5 +++++ 3 files changed, 26 insertions(+), 28 deletions(-) diff --git a/nix/libstore/build.cc b/nix/libstore/build.cc index f38cd29940..b3c994d6de 100644 --- a/nix/libstore/build.cc +++ b/nix/libstore/build.cc @@ -401,18 +401,6 @@ 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() @@ -2135,11 +2123,7 @@ void DerivationGoal::initChild() Strings envStrs; foreach (Environment::const_iterator, i, env) envStrs.push_back(rewriteHashes(i->first + "=" + i->second, rewritesToTmp)); - const char * * envArr = strings2CharPtrs(envStrs); - - Path program = drv.builder.c_str(); - std::vector args; /* careful with c_str()! */ - string user; /* must be here for its c_str()! */ + std::vector envArr = stringsToCharPtrs(envStrs); /* If we are running in `build-users' mode, then switch to the user we allocated above. Make sure that we drop all root @@ -2165,17 +2149,18 @@ 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).c_str()); - args.push_back(0); + args.push_back(rewriteHashes(*i, rewritesToTmp)); + std::vector argArr = stringsToCharPtrs(args); restoreSIGPIPE(); /* Execute the program. This should not return. */ inSetup = false; - execve(program.c_str(), (char * *) &args[0], (char * *) envArr); + execve(drv.builder.c_str(), (char * *) &argArr[0], (char * *) &envArr[0]); throw SysError(format("executing `%1%'") % drv.builder); @@ -2778,7 +2763,7 @@ void SubstitutionGoal::tryToRun() args.push_back("--substitute"); args.push_back(storePath); args.push_back(destPath); - const char * * argArr = strings2CharPtrs(args); + std::vector argArr = stringsToCharPtrs(args); /* Fork the substitute program. */ pid = maybeVfork(); @@ -2796,7 +2781,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); + execv(sub.c_str(), (char * *) &argArr[0]); throw SysError(format("executing `%1%'") % sub); diff --git a/nix/libutil/util.cc b/nix/libutil/util.cc index 846674a29d..024cea83d1 100644 --- a/nix/libutil/util.cc +++ b/nix/libutil/util.cc @@ -852,16 +852,20 @@ void killUser(uid_t uid) ////////////////////////////////////////////////////////////////////// +std::vector stringsToCharPtrs(const Strings & ss) +{ + std::vector 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 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(); @@ -880,6 +884,10 @@ 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 ce2d77c19a..a70981877b 100644 --- a/nix/libutil/util.hh +++ b/nix/libutil/util.hh @@ -257,6 +257,11 @@ 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 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 & exceptions); -- cgit v1.2.3