[PATCH] Use sq_quote() to properly quote the parameter to call shell.

This tries to be more lenient to the users and stricter to the
attackers by quoting the input properly for shell safety,
instead of forbidding certain characters from the input.

Things to note:

- We do not quote "prog" parameter (which comes from --exec).
The user should know what he is doing.  --exec='echo foo'
will supply the first two parameters to the resulting
command, while --exec="'echo foo'" will give the first
parameter, a single string with a space inside.

- We do not care too much about leaking the sq_quote() output
just before running exec().

Signed-off-by: Junio C Hamano <junkio@cox.net>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/connect.c b/connect.c
index 3d4b31d3405a4bb61f81a03fc1103ba2bea68411..b508be3ed8530fbc82d15ec2846e8b3c8dba9a1e 100644 (file)
--- a/connect.c
+++ b/connect.c
@@ -1,5 +1,6 @@
 #include "cache.h"
 #include "pkt-line.h"
 #include "cache.h"
 #include "pkt-line.h"

+#include "quote.h"

 #include <sys/wait.h>
 
 int get_ack(int fd, unsigned char *result_sha1)
 #include <sys/wait.h>
 
 int get_ack(int fd, unsigned char *result_sha1)


@@ -41,34 +42,6 @@ int path_match(const char *path, int nr, char **match)
        return 0;
 }
 
        return 0;
 }
 

-/*
- * First, make it shell-safe.  We do this by just disallowing any
- * special characters. Somebody who cares can do escaping and let
- * through the rest. But since we're doing to feed this to ssh as
- * a command line, we're going to be pretty damn anal for now.
- */
-static char *shell_safe(char *url)
-{
-       char *n = url;
-       unsigned char c;
-       static const char flags[256] = {
-               ['0'...'9'] = 1,
-               ['a'...'z'] = 1,
-               ['A'...'Z'] = 1,
-               ['.'] = 1, ['/'] = 1,
-               ['-'] = 1, ['+'] = 1,
-               [':'] = 1, ['_'] = 1,
-               ['@'] = 1, [','] = 1,
-               ['~'] = 1, ['^'] = 1,
-       };
-
-       while ((c = *n++) != 0) {
-               if (flags[c] != 1)
-                       die("I don't like '%c'. Sue me.", c);
-       }
-       return url;
-}
-

 /*
  * Yeah, yeah, fixme. Need to pass in the heads etc.
  */
 /*
  * Yeah, yeah, fixme. Need to pass in the heads etc.
  */


@@ -80,7 +53,6 @@ int git_connect(int fd[2], char *url, const char *prog)
        int pipefd[2][2];
        pid_t pid;
 
        int pipefd[2][2];
        pid_t pid;
 

-       url = shell_safe(url);

        host = NULL;
        path = url;
        colon = strchr(url, ':');
        host = NULL;
        path = url;
        colon = strchr(url, ':');


@@ -89,11 +61,12 @@ int git_connect(int fd[2], char *url, const char *prog)
                host = url;
                path = colon+1;
        }
                host = url;
                path = colon+1;
        }

-       snprintf(command, sizeof(command), "%s %s", prog, path);

        if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
                die("unable to create pipe pair for communication");
        pid = fork();
        if (!pid) {
        if (pipe(pipefd[0]) < 0 || pipe(pipefd[1]) < 0)
                die("unable to create pipe pair for communication");
        pid = fork();
        if (!pid) {

+               snprintf(command, sizeof(command), "%s %s", prog,
+                        sq_quote(path));

                dup2(pipefd[1][0], 0);
                dup2(pipefd[0][1], 1);
                close(pipefd[0][0]);
                dup2(pipefd[1][0], 0);
                dup2(pipefd[0][1], 1);
                close(pipefd[0][0]);