Junio C Hamano writes: > Anders Waldenborg <[email protected]> writes: > >> @@ -1352,6 +1353,17 @@ static size_t format_commit_one(struct strbuf *sb, /* >> in UTF-8 */ >> arg++; >> >> opts.only_trailers = 1; >> + } else if (skip_prefix(arg, "separator=", >> &arg)) { >> + size_t seplen = strcspn(arg, ",)"); >> + strbuf_reset(&sepbuf); >> + char *fmt = xstrndup(arg, seplen); >> + strbuf_expand(&sepbuf, fmt, >> format_fundamental, NULL); > > This somehow feels akin to using end-user supplied param to printf(3) > as its format argument e.g. > > int main(int ac, char *av) { > printf(av[1]); > return 0; > } > > which is not a good idea. Is there a mechanism with which we can > ensure that the separator=<what> specification will never come from > potentially malicious sources (e.g. not used to show things on webpage > allowing random folks who access he site to supply custom format)?
I can't see a case where this could add anything that isn't already possible. AFAICU strbuf_expand doesn't suffer from the worst things that printf(3) suffers from wrt untrusted format string (i.e no printf style %n which can write to memory, and no vaargs on stack which allows leaking random stuff). The separator option is part of the full format string. If a malicious user can specify that, they can't really do anything new, as the separator only can expand %n and %xNN, which they already can do in the full string. But maybe I'm missing something?
