From 090fd4fe24b9da9d912aa5856f4cd32d157924c6 Mon Sep 17 00:00:00 2001 From: Jeff King Date: Wed, 20 Feb 2013 15:01:26 -0500 Subject: upload-archive: use argv_array to store client arguments The current parsing scheme for upload-archive is to pack arguments into a fixed-size buffer, separated by NULs, and put a pointer to each argument in the buffer into a fixed-size argv array. This works fine, and the limits are high enough that nobody reasonable is going to hit them, but it makes the code hard to follow. Instead, let's just stuff the arguments into an argv_array, which is much simpler. That lifts the "all arguments must fit inside 4K together" limit. We could also trivially lift the MAX_ARGS limitation (in fact, we have to keep extra code to enforce it). But that would mean a client could force us to allocate an arbitrary amount of memory simply by sending us "argument" lines. By limiting the MAX_ARGS, we limit an attacker to about 4 megabytes (64 times a maximum 64K packet buffer). That may sound like a lot compared to the 4K limit, but it's not a big deal compared to what git-archive will actually allocate while working (e.g., to load blobs into memory). The important thing is that it is bounded. Signed-off-by: Jeff King Signed-off-by: Junio C Hamano diff --git a/builtin/upload-archive.c b/builtin/upload-archive.c index c3d134e..1517dec 100644 --- a/builtin/upload-archive.c +++ b/builtin/upload-archive.c @@ -7,6 +7,7 @@ #include "pkt-line.h" #include "sideband.h" #include "run-command.h" +#include "argv-array.h" static const char upload_archive_usage[] = "git upload-archive "; @@ -18,10 +19,9 @@ static const char deadchild[] = int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix) { - const char *sent_argv[MAX_ARGS]; + struct argv_array sent_argv = ARGV_ARRAY_INIT; const char *arg_cmd = "argument "; - char *p, buf[4096]; - int sent_argc; + char buf[4096]; int len; if (argc != 2) @@ -31,33 +31,26 @@ int cmd_upload_archive_writer(int argc, const char **argv, const char *prefix) die("'%s' does not appear to be a git repository", argv[1]); /* put received options in sent_argv[] */ - sent_argc = 1; - sent_argv[0] = "git-upload-archive"; - for (p = buf;;) { + argv_array_push(&sent_argv, "git-upload-archive"); + for (;;) { /* This will die if not enough free space in buf */ - len = packet_read_line(0, p, (buf + sizeof buf) - p); + len = packet_read_line(0, buf, sizeof(buf)); if (len == 0) break; /* got a flush */ - if (sent_argc > MAX_ARGS - 2) - die("Too many options (>%d)", MAX_ARGS - 2); + if (sent_argv.argc > MAX_ARGS) + die("Too many options (>%d)", MAX_ARGS - 1); - if (p[len-1] == '\n') { - p[--len] = 0; + if (buf[len-1] == '\n') { + buf[--len] = 0; } - if (len < strlen(arg_cmd) || - strncmp(arg_cmd, p, strlen(arg_cmd))) - die("'argument' token or flush expected"); - len -= strlen(arg_cmd); - memmove(p, p + strlen(arg_cmd), len); - sent_argv[sent_argc++] = p; - p += len; - *p++ = 0; + if (prefixcmp(buf, arg_cmd)) + die("'argument' token or flush expected"); + argv_array_push(&sent_argv, buf + strlen(arg_cmd)); } - sent_argv[sent_argc] = NULL; /* parse all options sent by the client */ - return write_archive(sent_argc, sent_argv, prefix, 0, NULL, 1); + return write_archive(sent_argv.argc, sent_argv.argv, prefix, 0, NULL, 1); } __attribute__((format (printf, 1, 2))) -- cgit v0.10.2-6-g49f6