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 <[EMAIL PROTECTED]>
---
This depends on the previous [PATCH] Make sq_expand() available as sq_quote().
Please discard the one I sent by mistake that lacks additions of
quote.[ch] files.
To verify the stuff is quoted properly, I used the following
command invocations. They show that the metacharacters are
passed unmolested by the intervening shell and ssh commands:
$ git-send-pack --exec=/bin/echo '0011 abc;d\ef g<h'
fatal: protocol error: expected sha/ref, got ' abc;d\ef g<h'
$ git-send-pack --exec=/bin/echo localhost:'0011 abc;d\ef g<h'
fatal: protocol error: expected sha/ref, got ' abc;d\ef g<h'
$ git-fetch-pack '0011 abc;d\ef g<h' a
fatal: git-upload-pack unable to chdir to 0011 abc;d\ef g<h
fatal: unexpected EOF
$ git-fetch-pack localhost:'0011 abc;d\ef g<h' a
fatal: git-upload-pack unable to chdir to 0011 abc;d\ef g<h
fatal: unexpected EOF
Also by storing the following executable script in
/var/tmp/j0.sh and using git-send-pack --exec=/var/tmp/j0.sh
with the same parameters:
#!/bin/sh
o=/var/tmp/j0.out
echo $$ >>$o
i=0
echo "0: [$0]" >>$o
for x
do
i=$(expr $i + 1)
echo "$i: [$x]" >>$o
done
it can be verified that the funny string is indeed passed as a
single parameter to it.
connect.c | 33 +++------------------------------
1 files changed, 3 insertions(+), 30 deletions(-)
e6e65f02919048f37467a9aca55ed19892dd2a7e
diff --git a/connect.c b/connect.c
--- a/connect.c
+++ b/connect.c
@@ -1,5 +1,6 @@
#include "cache.h"
#include "pkt-line.h"
+#include "quote.h"
#include <sys/wait.h>
int get_ack(int fd, unsigned char *result_sha1)
@@ -42,34 +43,6 @@ int path_match(const char *path, int nr,
}
/*
- * 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.
*/
int git_connect(int fd[2], char *url, const char *prog)
@@ -80,7 +53,6 @@ int git_connect(int fd[2], char *url, co
int pipefd[2][2];
pid_t pid;
- url = shell_safe(url);
host = NULL;
path = url;
colon = strchr(url, ':');
@@ -89,11 +61,12 @@ int git_connect(int fd[2], char *url, co
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) {
+ snprintf(command, sizeof(command), "%s %s", prog,
+ sq_quote(path));
dup2(pipefd[1][0], 0);
dup2(pipefd[0][1], 1);
close(pipefd[0][0]);
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [EMAIL PROTECTED]
More majordomo info at http://vger.kernel.org/majordomo-info.html
