[RFC/PATCH 4/5] daemon.c: accept extra service arguments

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Before 73bb33a (daemon: Strictly parse the "extra arg" part of the
command - 2009-06-04) a client sending extra arguments could DoS
git-daemon. 73bb33a fixed it by forbidding extra arguments.

Allow arguments other than "host=" again as a preparation step for
upload-pack2. "host=" if present must be the first argument
though. The remaining arguments are concatenated by whitespace.

So far none of supported services support extra arguments. Attempting
to do will abort the service, just like how it is before. We might
want to make them silently ignore extra arguments though.

Signed-off-by: Stefan Beller <sbeller@xxxxxxxxxx>
---
 connect.c |  3 ---
 daemon.c  | 37 ++++++++++++++++++++++++-------------
 2 files changed, 24 insertions(+), 16 deletions(-)

diff --git a/connect.c b/connect.c
index 7b6b241..97dd732 100644
--- a/connect.c
+++ b/connect.c
@@ -683,9 +683,6 @@ struct child_process *git_connect(int fd[2], const char *url,
 		/*
 		 * Separate original protocol components prog and path
 		 * from extended host header with a NUL byte.
-		 *
-		 * Note: Do not add any other headers here!  Doing so
-		 * will cause older git-daemon servers to crash.
 		 */
 		if (!service_flags)
 			packet_write(fd[1], "%s %s%chost=%s%c",
diff --git a/daemon.c b/daemon.c
index 54a03bd..c45d0d6 100644
--- a/daemon.c
+++ b/daemon.c
@@ -221,7 +221,7 @@ static const char *path_ok(const char *directory)
 	return NULL;		/* Fallthrough. Deny by default */
 }
 
-typedef int (*daemon_service_fn)(void);
+typedef int (*daemon_service_fn)(const char *);
 struct daemon_service {
 	const char *name;
 	const char *config_name;
@@ -302,7 +302,8 @@ error_return:
 	return -1;
 }
 
-static int run_service(const char *dir, struct daemon_service *service)
+static int run_service(const char *dir, struct daemon_service *service,
+		       const char *args)
 {
 	const char *path;
 	int enabled = service->enabled;
@@ -361,7 +362,7 @@ static int run_service(const char *dir, struct daemon_service *service)
 	 */
 	signal(SIGTERM, SIG_IGN);
 
-	return service->fn();
+	return service->fn(args);
 }
 
 static void copy_to_log(int fd)
@@ -403,27 +404,31 @@ static int run_service_command(const char **argv)
 	return finish_command(&cld);
 }
 
-static int upload_pack(void)
+static int upload_pack(const char *args)
 {
 	/* Timeout as string */
 	char timeout_buf[64];
-	const char *argv[] = { "upload-pack", "--strict", NULL, ".", NULL };
+	const char *argv[] = { "upload-pack", "--strict", NULL, ".", NULL, NULL };
 
 	argv[2] = timeout_buf;
+	argv[4] = args;
 
 	snprintf(timeout_buf, sizeof timeout_buf, "--timeout=%u", timeout);
 	return run_service_command(argv);
 }
 
-static int upload_archive(void)
+static int upload_archive(const char *args)
 {
 	static const char *argv[] = { "upload-archive", ".", NULL };
+	if (args)
+		die("invalid request");
 	return run_service_command(argv);
 }
 
-static int receive_pack(void)
+static int receive_pack(const char *args)
 {
-	static const char *argv[] = { "receive-pack", ".", NULL };
+	static const char *argv[] = { "receive-pack", ".", NULL, NULL };
+	argv[2] = args;
 	return run_service_command(argv);
 }
 
@@ -487,7 +492,7 @@ static void parse_host_and_port(char *hostport, char **host,
 /*
  * Read the host as supplied by the client connection.
  */
-static void parse_host_arg(char *extra_args, int buflen)
+static void parse_host_arg(char *extra_args, char **remaining_args, int buflen)
 {
 	char *val;
 	int vallen;
@@ -514,8 +519,13 @@ static void parse_host_arg(char *extra_args, int buflen)
 			/* On to the next one */
 			extra_args = val + vallen;
 		}
-		if (extra_args < end && *extra_args)
-			die("Invalid request");
+	}
+
+	if (remaining_args) {
+		for (val = extra_args; val < end; val++)
+			if (!*val)
+				*val = ' ';
+		*remaining_args = extra_args;
 	}
 
 	/*
@@ -577,6 +587,7 @@ static int execute(void)
 {
 	char *line = packet_buffer;
 	int pktlen, len, i;
+	char *args = NULL;
 	char *addr = getenv("REMOTE_ADDR"), *port = getenv("REMOTE_PORT");
 
 	if (addr)
@@ -603,7 +614,7 @@ static int execute(void)
 	hostname = canon_hostname = ip_address = tcp_port = NULL;
 
 	if (len != pktlen)
-		parse_host_arg(line + len + 1, pktlen - len - 1);
+		parse_host_arg(line + len + 1, &args, pktlen - len - 1);
 
 	for (i = 0; i < ARRAY_SIZE(daemon_service); i++) {
 		struct daemon_service *s = &(daemon_service[i]);
@@ -616,7 +627,7 @@ static int execute(void)
 			 * Note: The directory here is probably context sensitive,
 			 * and might depend on the actual service being performed.
 			 */
-			return run_service(arg, s);
+			return run_service(arg, s, args);
 		}
 	}
 
-- 
2.3.0.81.gc37f363

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]