archive: expand only a single %(describe) per archive

Every %(describe) placeholder in $Format:...$ strings in files with the attribute export-subst is expanded by calling git describe. This can potentially result in a lot of such calls per archive. That's OK for local repositories under control of the user of git archive, but could be a problem for hosted repositories. Expand only a single %(describe) placeholder per archive for now to avoid denial-of-service attacks. We can make this limit configurable later if needed, but let's start out simple. Reported-by: Jeff King <peff@peff.net> Signed-off-by: René Scharfe <l.s.r@web.de> Signed-off-by: Junio C Hamano <gitster@pobox.com>

René Scharfe committed Feb 28, 2021 at 12:22 UTC 96099726ddb00b45135964220ce56468ba9fe184
6 files changed +41 -7
Documentation/gitattributes.txt
+2 -1
@@ -1174,7 +1174,8 @@ tag then no replacement will be done. The placeholders are the same
1174 as those for the option `--pretty=format:` of linkgit:git-log[1],
1175 except that they need to be wrapped like this: `$Format:PLACEHOLDERS$`
1176 in the file. E.g. the string `$Format:%H$` will be replaced by the
1177 -commit hash.
1177 +commit hash. However, only one `%(describe)` placeholder is expanded
1178 +per archive to avoid denial-of-service attacks.
1179
1180
1181 Packing objects
archive.c
+10 -6
@@ -37,13 +37,10 @@ void init_archivers(void)
37
38 static void format_subst(const struct commit *commit,
39 const char *src, size_t len,
40 - struct strbuf *buf)
40 + struct strbuf *buf, struct pretty_print_context *ctx)
41 {
42 char *to_free = NULL;
43 struct strbuf fmt = STRBUF_INIT;
44 - struct pretty_print_context ctx = {0};
45 - ctx.date_mode.type = DATE_NORMAL;
46 - ctx.abbrev = DEFAULT_ABBREV;
44
45 if (src == buf->buf)
46 to_free = strbuf_detach(buf, NULL);
@@ -61,7 +58,7 @@ static void format_subst(const struct commit *commit,
58 strbuf_add(&fmt, b + 8, c - b - 8);
59
60 strbuf_add(buf, src, b - src);
64 - format_commit_message(commit, fmt.buf, buf, &ctx);
61 + format_commit_message(commit, fmt.buf, buf, ctx);
62 len -= c + 1 - src;
63 src = c + 1;
64 }
@@ -94,7 +91,7 @@ static void *object_file_to_archive(const struct archiver_args *args,
91 strbuf_attach(&buf, buffer, *sizep, *sizep + 1);
92 convert_to_working_tree(args->repo->index, path, buf.buf, buf.len, &buf, &meta);
93 if (commit)
97 - format_subst(commit, buf.buf, buf.len, &buf);
94 + format_subst(commit, buf.buf, buf.len, &buf, args->pretty_ctx);
95 buffer = strbuf_detach(&buf, &size);
96 *sizep = size;
97 }
@@ -633,12 +630,19 @@ int write_archive(int argc, const char **argv, const char *prefix,
630 const char *name_hint, int remote)
631 {
632 const struct archiver *ar = NULL;
633 + struct pretty_print_describe_status describe_status = {0};
634 + struct pretty_print_context ctx = {0};
635 struct archiver_args args;
636 int rc;
637
638 git_config_get_bool("uploadarchive.allowunreachable", &remote_allow_unreachable);
639 git_config(git_default_config, NULL);
640
641 + describe_status.max_invocations = 1;
642 + ctx.date_mode.type = DATE_NORMAL;
643 + ctx.abbrev = DEFAULT_ABBREV;
644 + ctx.describe_status = &describe_status;
645 + args.pretty_ctx = &ctx;
646 args.repo = repo;
647 args.prefix = prefix;
648 string_list_init(&args.extra_files, 1);
archive.h
+2
@@ -5,6 +5,7 @@
5 #include "pathspec.h"
6
7 struct repository;
8 +struct pretty_print_context;
9
10 struct archiver_args {
11 struct repository *repo;
@@ -22,6 +23,7 @@ struct archiver_args {
23 unsigned int convert : 1;
24 int compression_level;
25 struct string_list extra_files;
26 + struct pretty_print_context *pretty_ctx;
27 };
28
29 /* main api */
pretty.c
+8
@@ -1247,6 +1247,14 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
1247 struct child_process cmd = CHILD_PROCESS_INIT;
1248 struct strbuf out = STRBUF_INIT;
1249 struct strbuf err = STRBUF_INIT;
1250 + struct pretty_print_describe_status *describe_status;
1251 +
1252 + describe_status = c->pretty_ctx->describe_status;
1253 + if (describe_status) {
1254 + if (!describe_status->max_invocations)
1255 + return 0;
1256 + describe_status->max_invocations--;
1257 + }
1258
1259 cmd.git_cmd = 1;
1260 strvec_push(&cmd.args, "describe");
pretty.h
+5
@@ -23,6 +23,10 @@ enum cmit_fmt {
23 CMIT_FMT_UNSPECIFIED
24 };
25
26 +struct pretty_print_describe_status {
27 + unsigned int max_invocations;
28 +};
29 +
30 struct pretty_print_context {
31 /*
32 * Callers should tweak these to change the behavior of pp_* functions.
@@ -44,6 +48,7 @@ struct pretty_print_context {
48 int color;
49 struct ident_split *from_ident;
50 unsigned encode_email_headers:1;
51 + struct pretty_print_describe_status *describe_status;
52
53 /*
54 * Fields below here are manipulated internally by pp_* functions and
t/t5001-archive-attr.sh
+14
@@ -128,4 +128,18 @@ test_expect_success 'export-subst' '
128 test_cmp substfile2 archive/substfile2
129 '
130
131 +test_expect_success 'export-subst expands %(describe) once' '
132 + echo "\$Format:%(describe)\$" >substfile3 &&
133 + echo "\$Format:%(describe)\$" >>substfile3 &&
134 + echo "\$Format:%(describe)${LF}%(describe)\$" >substfile4 &&
135 + git add substfile[34] &&
136 + git commit -m export-subst-describe &&
137 + git tag -m export-subst-describe export-subst-describe &&
138 + git archive HEAD >archive-describe.tar &&
139 + extract_tar_to_dir archive-describe &&
140 + desc=$(git describe) &&
141 + grep -F "$desc" archive-describe/substfile[34] >substituted &&
142 + test_line_count = 1 substituted
143 +'
144 +
145 test_done