Am 07.02.2010 08:10, schrieb Jonathan Nieder:
> ‘git archive --format=<string of 25 characters or more>’ overflows a
> local buffer, producing a segfault here.
>
> The context: in commit 0f4b377 (git-archive: infer output format from
> filename when unspecified, 2009-09-14), the cmd_archive wrapper
> learned to produce a format argument for the local or remote archive
> machinery in a local buffer, but that code was missing a bounds check.
>
> So add the missing check. As a belt-and-suspenders measure, also use
> snprintf to make sure the copy afterwards does not overflow.
>
> Cc: Rene Scharfe <rene.scharfe@xxxxxxxxxxxxxx>
> Cc: Dmitry Potapov <dpotapov@xxxxxxxxx>
> Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx>
> ---
> I noticed this while reading over the archive code. Thoughts?
>
> builtin-archive.c | 4 +++-
> t/t5000-tar-tree.sh | 6 ++++++
> 2 files changed, 9 insertions(+), 1 deletions(-)
>
> diff --git a/builtin-archive.c b/builtin-archive.c
> index 3fb4136..94db00d 100644
> --- a/builtin-archive.c
> +++ b/builtin-archive.c
> @@ -107,7 +107,9 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
> }
>
> if (format) {
> - sprintf(fmt_opt, "--format=%s", format);
> + if (strlen(format) > sizeof(fmt_opt) - sizeof("--format="))
> + die("git archive: format is too long: %.50s", format);
> + snprintf(fmt_opt, sizeof(fmt_opt), "--format=%s", format);
> /*
> * We have enough room in argv[] to muck it in place,
> * because either --format and/or --output must have
Thanks. I think this should go into 1.7.0. I'd use the same format
string for the error message as archive.c, i.e. "Unknown archive format
'%s'". Later I'd rather do this:
builtin-archive.c | 25 ++++++++++++-------------
1 files changed, 12 insertions(+), 13 deletions(-)
diff --git a/builtin-archive.c b/builtin-archive.c
index 3fb4136..ffe4f4a 100644
--- a/builtin-archive.c
+++ b/builtin-archive.c
@@ -70,7 +70,7 @@ static const char *format_from_name(const char *filename)
return NULL;
ext++;
if (!strcasecmp(ext, "zip"))
- return "zip";
+ return "--format=zip";
return NULL;
}
@@ -92,33 +92,32 @@ int cmd_archive(int argc, const char **argv, const char *prefix)
"retrieve the archive from remote repository <repo>"),
OPT_STRING(0, "exec", &exec, "cmd",
"path to the remote git-upload-archive command"),
- OPT_STRING(0, "format", &format, "fmt", "archive format"),
OPT_END()
};
- char fmt_opt[32];
argc = parse_options(argc, argv, prefix, local_opts, NULL,
PARSE_OPT_KEEP_ALL);
if (output) {
create_output_file(output);
- if (!format)
- format = format_from_name(output);
+ format = format_from_name(output);
}
if (format) {
- sprintf(fmt_opt, "--format=%s", format);
/*
* We have enough room in argv[] to muck it in place,
- * because either --format and/or --output must have
- * been given on the original command line if we get
- * to this point, and parse_options() must have eaten
- * it, i.e. we can add back one element to the array.
- * But argv[] may contain "--"; we should make it the
- * first option.
+ * because --output must have been given on the
+ * original command line if we get to this point, and
+ * parse_options() must have eaten it, i.e. we can add
+ * back one element to the array. We add a fake
+ * --format option at the beginning with the hint
+ * derived from our output filename. This way explicit
+ * --format options can override it, and the fake
+ * option is inserted before any "--" that might have
+ * been given.
*/
memmove(argv + 2, argv + 1, sizeof(*argv) * argc);
- argv[1] = fmt_opt;
+ argv[1] = format;
argv[++argc] = NULL;
}
--
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