From 6e43b2a1cd1ea487fef86efe607b833bc304e7ef Mon Sep 17 00:00:00 2001 From: Andrew Ayer Date: Tue, 10 Jun 2014 21:21:38 -0700 Subject: [PATCH] New exec_command() that takes command as array instead of string This abstracts away the details of argument quoting, which differs between Unix and Windows. Also replace all uses of the system() library call with exec_command(). Although system() exists on Windows, it executes the command via cmd.exe, which has ridiculous escaping rules. --- commands.cpp | 180 +++++++++++++++++++++++++++++++------------------ gpg.cpp | 56 ++++++++++----- util-unix.cpp | 41 +++++++++-- util-win32.cpp | 9 ++- util.hpp | 6 +- 5 files changed, 200 insertions(+), 92 deletions(-) diff --git a/commands.cpp b/commands.cpp index 1cc47ef..9519eb1 100644 --- a/commands.cpp +++ b/commands.cpp @@ -47,44 +47,43 @@ #include #include -static void configure_git_filters () +static void git_config (const std::string& name, const std::string& value) { - std::string git_crypt_path(our_exe_path()); + std::vector command; + command.push_back("git"); + command.push_back("config"); + command.push_back(name); + command.push_back(value); - // git config filter.git-crypt.smudge "/path/to/git-crypt smudge" - std::string command("git config filter.git-crypt.smudge "); - command += escape_shell_arg(escape_shell_arg(git_crypt_path) + " smudge"); - - if (!successful_exit(system(command.c_str()))) { - throw Error("'git config' failed"); - } - - // git config filter.git-crypt.clean "/path/to/git-crypt clean" - command = "git config filter.git-crypt.clean "; - command += escape_shell_arg(escape_shell_arg(git_crypt_path) + " clean"); - - if (!successful_exit(system(command.c_str()))) { - throw Error("'git config' failed"); - } - - // git config diff.git-crypt.textconv "/path/to/git-crypt diff" - command = "git config diff.git-crypt.textconv "; - command += escape_shell_arg(escape_shell_arg(git_crypt_path) + " diff"); - - if (!successful_exit(system(command.c_str()))) { + if (!successful_exit(exec_command(command))) { throw Error("'git config' failed"); } } +static void configure_git_filters () +{ + std::string escaped_git_crypt_path(escape_shell_arg(our_exe_path())); + + git_config("filter.git-crypt.smudge", escaped_git_crypt_path + " smudge"); + git_config("filter.git-crypt.clean", escaped_git_crypt_path + " clean"); + git_config("diff.git-crypt.textconv", escaped_git_crypt_path + " diff"); +} + static std::string get_internal_key_path () { - std::stringstream output; + // git rev-parse --git-dir + std::vector command; + command.push_back("git"); + command.push_back("rev-parse"); + command.push_back("--git-dir"); - if (!successful_exit(exec_command("git rev-parse --git-dir", output))) { + std::stringstream output; + + if (!successful_exit(exec_command(command, output))) { throw Error("'git rev-parse --git-dir' failed - is this a Git repository?"); } - std::string path; + std::string path; std::getline(output, path); path += "/git-crypt/key"; return path; @@ -92,13 +91,19 @@ static std::string get_internal_key_path () static std::string get_repo_keys_path () { - std::stringstream output; + // git rev-parse --show-toplevel + std::vector command; + command.push_back("git"); + command.push_back("rev-parse"); + command.push_back("--show-toplevel"); - if (!successful_exit(exec_command("git rev-parse --show-toplevel", output))) { + std::stringstream output; + + if (!successful_exit(exec_command(command, output))) { throw Error("'git rev-parse --show-toplevel' failed - is this a Git repository?"); } - std::string path; + std::string path; std::getline(output, path); if (path.empty()) { @@ -110,6 +115,52 @@ static std::string get_repo_keys_path () return path; } +static std::string get_path_to_top () +{ + // git rev-parse --show-cdup + std::vector command; + command.push_back("git"); + command.push_back("rev-parse"); + command.push_back("--show-cdup"); + + std::stringstream output; + + if (!successful_exit(exec_command(command, output))) { + throw Error("'git rev-parse --show-cdup' failed - is this a Git repository?"); + } + + std::string path_to_top; + std::getline(output, path_to_top); + + return path_to_top; +} + +static void get_git_status (std::ostream& output) +{ + // git status -uno --porcelain + std::vector command; + command.push_back("git"); + command.push_back("status"); + command.push_back("-uno"); // don't show untracked files + command.push_back("--porcelain"); + + if (!successful_exit(exec_command(command, output))) { + throw Error("'git status' failed - is this a Git repository?"); + } +} + +static bool check_if_head_exists () +{ + // git rev-parse HEAD + std::vector command; + command.push_back("git"); + command.push_back("rev-parse"); + command.push_back("HEAD"); + + std::stringstream output; + return successful_exit(exec_command(command, output)); +} + static void load_key (Key_file& key_file, const char* legacy_path =0) { if (legacy_path) { @@ -421,20 +472,20 @@ int unlock (int argc, char** argv) return 2; } - // 0. Check to see if HEAD exists. See below why we do this. - bool head_exists = successful_exit(system("git rev-parse HEAD >/dev/null 2>/dev/null")); - - // 1. Make sure working directory is clean (ignoring untracked files) + // 0. Make sure working directory is clean (ignoring untracked files) // We do this because we run 'git checkout -f HEAD' later and we don't // want the user to lose any changes. 'git checkout -f HEAD' doesn't touch // untracked files so it's safe to ignore those. - int status; + + // Running 'git status' also serves as a check that the Git repo is accessible. + std::stringstream status_output; - status = exec_command("git status -uno --porcelain", status_output); - if (!successful_exit(status)) { - std::clog << "Error: 'git status' failed - is this a git repository?" << std::endl; - return 1; - } else if (status_output.peek() != -1 && head_exists) { + get_git_status(status_output); + + // 1. Check to see if HEAD exists. See below why we do this. + bool head_exists = check_if_head_exists(); + + if (status_output.peek() != -1 && head_exists) { // We only care that the working directory is dirty if HEAD exists. // If HEAD doesn't exist, we won't be resetting to it (see below) so // it doesn't matter that the working directory is dirty. @@ -446,11 +497,7 @@ int unlock (int argc, char** argv) // 2. Determine the path to the top of the repository. We pass this as the argument // to 'git checkout' below. (Determine the path now so in case it fails we haven't already // mucked with the git config.) - std::stringstream cdup_output; - if (!successful_exit(exec_command("git rev-parse --show-cdup", cdup_output))) { - std::clog << "Error: 'git rev-parse --show-cdup' failed" << std::endl; - return 1; - } + std::string path_to_top(get_path_to_top()); // 3. Install the key Key_file key_file; @@ -504,17 +551,20 @@ int unlock (int argc, char** argv) // If HEAD doesn't exist (perhaps because this repo doesn't have any files yet) // just skip the checkout. if (head_exists) { - std::string path_to_top; - std::getline(cdup_output, path_to_top); - - std::string command("git checkout -f HEAD -- "); + // git checkout -f HEAD -- path/to/top + std::vector command; + command.push_back("git"); + command.push_back("checkout"); + command.push_back("-f"); + command.push_back("HEAD"); + command.push_back("--"); if (path_to_top.empty()) { - command += "."; + command.push_back("."); } else { - command += escape_shell_arg(path_to_top); + command.push_back(path_to_top); } - if (!successful_exit(system(command.c_str()))) { + if (!successful_exit(exec_command(command))) { std::clog << "Error: 'git checkout' failed" << std::endl; std::clog << "git-crypt has been set up but existing encrypted files have not been decrypted" << std::endl; return 1; @@ -563,13 +613,12 @@ int add_collab (int argc, char** argv) // add/commit the new files if (!new_files.empty()) { - // git add ... - std::string command("git add"); - for (std::vector::const_iterator file(new_files.begin()); file != new_files.end(); ++file) { - command += " "; - command += escape_shell_arg(*file); - } - if (!successful_exit(system(command.c_str()))) { + // git add NEW_FILE ... + std::vector command; + command.push_back("git"); + command.push_back("add"); + command.insert(command.end(), new_files.begin(), new_files.end()); + if (!successful_exit(exec_command(command))) { std::clog << "Error: 'git add' failed" << std::endl; return 1; } @@ -582,14 +631,15 @@ int add_collab (int argc, char** argv) commit_message_builder << '\t' << gpg_shorten_fingerprint(*collab) << ' ' << gpg_get_uid(*collab) << '\n'; } - command = "git commit -m "; - command += escape_shell_arg(commit_message_builder.str()); - for (std::vector::const_iterator file(new_files.begin()); file != new_files.end(); ++file) { - command += " "; - command += escape_shell_arg(*file); - } + // git commit -m MESSAGE NEW_FILE ... + command.clear(); + command.push_back("git"); + command.push_back("commit"); + command.push_back("-m"); + command.push_back(commit_message_builder.str()); + command.insert(command.end(), new_files.begin(), new_files.end()); - if (!successful_exit(system(command.c_str()))) { + if (!successful_exit(exec_command(command))) { std::clog << "Error: 'git commit' failed" << std::endl; return 1; } diff --git a/gpg.cpp b/gpg.cpp index 05db9fb..4813b35 100644 --- a/gpg.cpp +++ b/gpg.cpp @@ -61,10 +61,15 @@ std::string gpg_shorten_fingerprint (const std::string& fingerprint) std::string gpg_get_uid (const std::string& fingerprint) { // gpg --batch --with-colons --fixed-list-mode --list-keys 0x7A399B2DB06D039020CD1CE1D0F3702D61489532 - std::string command("gpg --batch --with-colons --fixed-list-mode --list-keys "); - command += escape_shell_arg("0x" + fingerprint); + std::vector command; + command.push_back("gpg"); + command.push_back("--batch"); + command.push_back("--with-colons"); + command.push_back("--fixed-list-mode"); + command.push_back("--list-keys"); + command.push_back("0x" + fingerprint); std::stringstream command_output; - if (!successful_exit(exec_command(command.c_str(), command_output))) { + if (!successful_exit(exec_command(command, command_output))) { // This could happen if the keyring does not contain a public key with this fingerprint return ""; } @@ -88,10 +93,15 @@ std::vector gpg_lookup_key (const std::string& query) std::vector fingerprints; // gpg --batch --with-colons --fingerprint --list-keys jsmith@example.com - std::string command("gpg --batch --with-colons --fingerprint --list-keys "); - command += escape_shell_arg(query); + std::vector command; + command.push_back("gpg"); + command.push_back("--batch"); + command.push_back("--with-colons"); + command.push_back("--fingerprint"); + command.push_back("--list-keys"); + command.push_back(query); std::stringstream command_output; - if (successful_exit(exec_command(command.c_str(), command_output))) { + if (successful_exit(exec_command(command, command_output))) { while (command_output.peek() != -1) { std::string line; std::getline(command_output, line); @@ -109,8 +119,14 @@ std::vector gpg_lookup_key (const std::string& query) std::vector gpg_list_secret_keys () { // gpg --batch --with-colons --list-secret-keys --fingerprint + std::vector command; + command.push_back("gpg"); + command.push_back("--batch"); + command.push_back("--with-colons"); + command.push_back("--list-secret-keys"); + command.push_back("--fingerprint"); std::stringstream command_output; - if (!successful_exit(exec_command("gpg --batch --with-colons --list-secret-keys --fingerprint", command_output))) { + if (!successful_exit(exec_command(command, command_output))) { throw Gpg_error("gpg --list-secret-keys failed"); } @@ -132,22 +148,28 @@ std::vector gpg_list_secret_keys () void gpg_encrypt_to_file (const std::string& filename, const std::string& recipient_fingerprint, const char* p, size_t len) { // gpg --batch -o FILENAME -r RECIPIENT -e - std::string command("gpg --batch -o "); - command += escape_shell_arg(filename); - command += " -r "; - command += escape_shell_arg("0x" + recipient_fingerprint); - command += " -e"; - if (!successful_exit(exec_command_with_input(command.c_str(), p, len))) { + std::vector command; + command.push_back("gpg"); + command.push_back("--batch"); + command.push_back("-o"); + command.push_back(filename); + command.push_back("-r"); + command.push_back("0x" + recipient_fingerprint); + command.push_back("-e"); + if (!successful_exit(exec_command_with_input(command, p, len))) { throw Gpg_error("Failed to encrypt"); } } void gpg_decrypt_from_file (const std::string& filename, std::ostream& output) { - // gpg -q -d - std::string command("gpg -q -d "); - command += escape_shell_arg(filename); - if (!successful_exit(exec_command(command.c_str(), output))) { + // gpg -q -d FILENAME + std::vector command; + command.push_back("gpg"); + command.push_back("-q"); + command.push_back("-d"); + command.push_back(filename); + if (!successful_exit(exec_command(command, output))) { throw Gpg_error("Failed to decrypt"); } } diff --git a/util-unix.cpp b/util-unix.cpp index 214f501..7f92a58 100644 --- a/util-unix.cpp +++ b/util-unix.cpp @@ -156,7 +156,36 @@ std::string our_exe_path () } } -int exec_command (const char* command, std::ostream& output) +static int execvp (const std::string& file, const std::vector& args) +{ + std::vector args_c_str; + args_c_str.reserve(args.size()); + for (std::vector::const_iterator arg(args.begin()); arg != args.end(); ++arg) { + args_c_str.push_back(arg->c_str()); + } + args_c_str.push_back(NULL); + return execvp(file.c_str(), const_cast(&args_c_str[0])); +} + +int exec_command (const std::vector& command) +{ + pid_t child = fork(); + if (child == -1) { + throw System_error("fork", "", errno); + } + if (child == 0) { + execvp(command[0], command); + perror(command[0].c_str()); + _exit(-1); + } + int status = 0; + if (waitpid(child, &status, 0) == -1) { + throw System_error("waitpid", "", errno); + } + return status; +} + +int exec_command (const std::vector& command, std::ostream& output) { int pipefd[2]; if (pipe(pipefd) == -1) { @@ -175,8 +204,8 @@ int exec_command (const char* command, std::ostream& output) dup2(pipefd[1], 1); close(pipefd[1]); } - execl("/bin/sh", "sh", "-c", command, NULL); - perror("/bin/sh"); + execvp(command[0], command); + perror(command[0].c_str()); _exit(-1); } close(pipefd[1]); @@ -198,7 +227,7 @@ int exec_command (const char* command, std::ostream& output) return status; } -int exec_command_with_input (const char* command, const char* p, size_t len) +int exec_command_with_input (const std::vector& command, const char* p, size_t len) { int pipefd[2]; if (pipe(pipefd) == -1) { @@ -217,8 +246,8 @@ int exec_command_with_input (const char* command, const char* p, size_t len) dup2(pipefd[0], 0); close(pipefd[0]); } - execl("/bin/sh", "sh", "-c", command, NULL); - perror("/bin/sh"); + execvp(command[0], command); + perror(command[0].c_str()); _exit(-1); } close(pipefd[0]); diff --git a/util-win32.cpp b/util-win32.cpp index d34e635..b758fc5 100644 --- a/util-win32.cpp +++ b/util-win32.cpp @@ -102,12 +102,17 @@ std::string our_exe_path () // TODO return argv0; } -int exec_command (const char* command, std::ostream& output) // TODO +int exec_command (const std::vector& command) // TODO { return -1; } -int exec_command_with_input (const char* command, const char* p, size_t len) // TODO +int exec_command (const std::vector& command, std::ostream& output) // TODO +{ + return -1; +} + +int exec_command_with_input (const std::vector& command, const char* p, size_t len) // TODO { return -1; } diff --git a/util.hpp b/util.hpp index 2637098..7cae193 100644 --- a/util.hpp +++ b/util.hpp @@ -36,6 +36,7 @@ #include #include #include +#include struct System_error { std::string action; @@ -58,8 +59,9 @@ public: void mkdir_parent (const std::string& path); // Create parent directories of path, __but not path itself__ std::string our_exe_path (); -int exec_command (const char* command, std::ostream& output); -int exec_command_with_input (const char* command, const char* p, size_t len); +int exec_command (const std::vector&); +int exec_command (const std::vector&, std::ostream& output); +int exec_command_with_input (const std::vector&, const char* p, size_t len); bool successful_exit (int status); std::string escape_shell_arg (const std::string&); uint32_t load_be32 (const unsigned char*);