From: Jeff King <peff@peff.net> To: git@vger.kernel.org Subject: [PATCH 5/6] config: use size_t to store parsed variable baselen Date: Fri, 10 Apr 2020 15:47:51 -0400 [thread overview] Message-ID: <20200410194751.GE1363756@coredump.intra.peff.net> (raw) In-Reply-To: <20200410194211.GA1363484@coredump.intra.peff.net> Most of the config parsing infrastructure is limited in what it can parse only by the size of memory, because it parses character by character, building up strbufs for keys, values, etc. One exception is the "baselen" value we keep in git_parse_source(), which is an int. That stores the length of the section.subsection base, to which we can then append individual key names (by truncating back to the baselen with strbuf_setlen(), and then appending characters for the key name). But because it's an int, if we see an absurdly long section or subsection, we may overflow the integer, wrapping negative. That negative value is then implicitly cast to a size_t when we pass it to strbuf_setlen(), creating a very large value and triggering a BUG. For example: $ { printf '[foo "' perl -e 'print "a" x 2**31' echo '"]bar = value' } >huge $ git config --file=huge --list fatal: BUG: strbuf_setlen() beyond buffer While this is obviously a silly case that we don't care about supporting, it's worth fixing it by switching to a size_t for a few reasons: - we should try to avoid hitting BUG assertions at all - avoiding integer truncation or overflow sets a good example and makes it easier to audit the code for more important issues - the BUG outcome is what happens in _this_ instance, because we wrap negative. If we used a 2**32 subsection, we'd wrap to a small positive value and actually generate wrong output (the subsection of our key would be truncated). Signed-off-by: Jeff King <peff@peff.net> --- config.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/config.c b/config.c index c48bb35dc0..1c25c94863 100644 --- a/config.c +++ b/config.c @@ -729,7 +729,7 @@ static int git_parse_source(config_fn_t fn, void *data, const struct config_options *opts) { int comment = 0; - int baselen = 0; + size_t baselen = 0; struct strbuf *var = &cf->var; int error_return = 0; char *error_msg = NULL; -- 2.26.0.414.ge3a6455e3d
next prev parent reply other threads:[~2020-04-10 19:47 UTC|newest] Thread overview: 16+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-04-10 19:42 [PATCH 0/6] better handling of gigantic config files Jeff King 2020-04-10 19:43 ` [PATCH 1/6] remote: drop auto-strlen behavior of make_branch() and make_rewrite() Jeff King 2020-04-10 21:44 ` Junio C Hamano 2020-04-10 19:44 ` [PATCH 2/6] parse_config_key(): return subsection len as size_t Jeff King 2020-04-10 19:44 ` [PATCH 3/6] config: drop useless length variable in write_pair() Jeff King 2020-04-10 21:51 ` Junio C Hamano 2020-04-10 19:46 ` [PATCH 4/6] git_config_parse_key(): return baselen as size_t Jeff King 2020-04-10 19:47 ` Jeff King [this message] 2020-04-10 19:50 ` [PATCH 6/6] config: reject parsing of files over INT_MAX Jeff King 2020-04-10 22:04 ` Junio C Hamano 2020-04-10 22:15 ` Jeff King 2020-04-13 0:47 ` Taylor Blau 2020-04-13 22:14 ` Junio C Hamano 2020-04-13 0:49 ` [PATCH 0/6] better handling of gigantic config files Taylor Blau 2020-04-13 17:20 ` Jeff King 2020-04-13 17:23 ` Taylor Blau
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style List information: http://vger.kernel.org/majordomo-info.html * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20200410194751.GE1363756@coredump.intra.peff.net \ --to=peff@peff.net \ --cc=git@vger.kernel.org \ --subject='Re: [PATCH 5/6] config: use size_t to store parsed variable baselen' \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: link
Code repositories for project(s) associated with this inbox: https://80x24.org/mirrors/git.git This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).