From 09f2825147d44bd0333f7f47d8ab3837f0848867 Mon Sep 17 00:00:00 2001 From: Jim Meyering Date: Mon, 12 Mar 2007 13:11:29 +0100 Subject: [PATCH] git-grep: don't use sscanf If you use scanf or sscanf to parse integers, your code probably accepts bogus inputs. For example, builtin-grep (aka git-grep) uses sscanf(scan, "%u", &num) to parse the integer argument to -A, -B, -C. Currently, "-C 1,000" and "-C 4294967297" are both treated just like "-C 1": $ git-grep -h -C 4294967297 juggle out and you may find it easier to switch back and forth if you juggle multiple lines of development simultaneously. Of course, you will pay the price of more disk usage to hold The obvious fix is to use strtoul instead. But using a bare strtoul is too messy, at least when done properly, so I've added a wrapper function. The new function in the patch below belongs elsewhere if it would be useful in replacing any of the four remaining uses of sscanf. One final note: With this change, I get a slightly different diagnostic depending on the context size: $ ./git-grep -h -C 4294967296 juggle fatal: 4294967296: invalid context length argument [Exit 128] $ ./git-grep -h -C 4294967295 juggle grep: 4294967295: invalid context length argument [Exit 1] A common convention that makes it easy to identify the source of a diagnostic is to include the program name before the first ":". Whether that should be "git" or "git-grep" is another question. Using "grep" or "fatal" is misleading. Signed-off-by: Jim Meyering Signed-off-by: Junio C Hamano --- builtin-grep.c | 15 ++++++++++++++- 1 file changed, 14 insertions(+), 1 deletion(-) diff --git a/builtin-grep.c b/builtin-grep.c index 694da5ba09..4510d35324 100644 --- a/builtin-grep.c +++ b/builtin-grep.c @@ -431,6 +431,19 @@ static const char emsg_missing_context_len[] = static const char emsg_missing_argument[] = "option requires an argument -%s"; +static int strtoul_ui(char const *s, unsigned int *result) +{ + unsigned long ul; + char *p; + + errno = 0; + ul = strtoul(s, &p, 10); + if (errno || *p || p == s || (unsigned int) ul != ul) + return -1; + *result = ul; + return 0; +} + int cmd_grep(int argc, const char **argv, const char *prefix) { int hit = 0; @@ -553,7 +566,7 @@ int cmd_grep(int argc, const char **argv, const char *prefix) scan = arg + 1; break; } - if (sscanf(scan, "%u", &num) != 1) + if (strtoul_ui(scan, &num)) die(emsg_invalid_context_len, scan); switch (arg[1]) { case 'A':