Skip to content

Commit 73bb33a

Browse files
spearcegitster
authored andcommitted
daemon: Strictly parse the "extra arg" part of the command
Since 1.4.4.5 (49ba83f "Add virtualization support to git-daemon") git daemon enters an infinite loop and never terminates if a client hides any extra arguments in the initial request line which is not exactly "\0host=blah\0". Since that change, a client must never insert additional extra arguments, or attempt to use any argument other than "host=", as any daemon will get stuck parsing the request line and will never complete the request. Since the client can't tell if the daemon is patched or not, it is not possible to know if additional extra args might actually be able to be safely requested. If we ever need to extend the git daemon protocol to support a new feature, we may have to do something like this to the exchange: # If both support git:// v2 # C: 000cgit://v2 S: 0010ok host user C: 0018host git.kernel.org C: 0027git-upload-pack /pub/linux-2.6.git S: ...git-upload-pack header... # If client supports git:// v2, server does not: # C: 000cgit://v2 S: <EOF> C: 003bgit-upload-pack /pub/linux-2.6.git\0host=git.kernel.org\0 S: ...git-upload-pack header... This requires the client to create two TCP connections to talk to an older git daemon, however all daemons since the introduction of daemon.c will safely reject the unknown "git://v2" command request, so the client can quite easily determine the server supports an older protocol. Signed-off-by: Shawn O. Pearce <[email protected]> Signed-off-by: Junio C Hamano <[email protected]>
1 parent 6c7f58d commit 73bb33a

File tree

2 files changed

+10
-5
lines changed

2 files changed

+10
-5
lines changed

connect.c

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -579,7 +579,10 @@ struct child_process *git_connect(int fd[2], const char *url_orig,
579579
git_tcp_connect(fd, host, flags);
580580
/*
581581
* Separate original protocol components prog and path
582-
* from extended components with a NUL byte.
582+
* from extended host header with a NUL byte.
583+
*
584+
* Note: Do not add any other headers here! Doing so
585+
* will cause older git-daemon servers to crash.
583586
*/
584587
packet_write(fd[1],
585588
"%s %s%chost=%s%c",

daemon.c

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -406,15 +406,15 @@ static char *xstrdup_tolower(const char *str)
406406
}
407407

408408
/*
409-
* Separate the "extra args" information as supplied by the client connection.
409+
* Read the host as supplied by the client connection.
410410
*/
411-
static void parse_extra_args(char *extra_args, int buflen)
411+
static void parse_host_arg(char *extra_args, int buflen)
412412
{
413413
char *val;
414414
int vallen;
415415
char *end = extra_args + buflen;
416416

417-
while (extra_args < end && *extra_args) {
417+
if (extra_args < end && *extra_args) {
418418
saw_extended_args = 1;
419419
if (strncasecmp("host=", extra_args, 5) == 0) {
420420
val = extra_args + 5;
@@ -436,6 +436,8 @@ static void parse_extra_args(char *extra_args, int buflen)
436436
/* On to the next one */
437437
extra_args = val + vallen;
438438
}
439+
if (extra_args < end && *extra_args)
440+
die("Invalid request");
439441
}
440442

441443
/*
@@ -545,7 +547,7 @@ static int execute(struct sockaddr *addr)
545547
hostname = canon_hostname = ip_address = tcp_port = NULL;
546548

547549
if (len != pktlen)
548-
parse_extra_args(line + len + 1, pktlen - len - 1);
550+
parse_host_arg(line + len + 1, pktlen - len - 1);
549551

550552
for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
551553
struct daemon_service *s = &(daemon_service[i]);

0 commit comments

Comments
 (0)