From e91cfe6085c4a61372d1f800b473b73b8d225d0d Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Thu, 6 Apr 2023 14:28:53 -0400 Subject: config.c: avoid integer truncation in `copy_or_rename_section_in_file()` There are a couple of spots within `copy_or_rename_section_in_file()` that incorrectly use an `int` to track an offset within a string, which may truncate or wrap around to a negative value. Historically it was impossible to have a line longer than 1024 bytes anyway, since we used fgets() with a fixed-size buffer of exactly that length. But the recent change to use a strbuf permits us to read lines of arbitrary length, so it's possible for a malicious input to cause us to overflow past INT_MAX and do an out-of-bounds array read. Practically speaking, however, this should never happen, since it requires 2GB section names or values, which are unrealistic in non-malicious circumstances. Co-authored-by: Jeff King Signed-off-by: Jeff King Signed-off-by: Taylor Blau --- config.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/config.c b/config.c index 524347676d..e4189aa2d7 100644 --- a/config.c +++ b/config.c @@ -3027,9 +3027,10 @@ void git_config_set_multivar(const char *key, const char *value, flags); } -static int section_name_match (const char *buf, const char *name) +static size_t section_name_match (const char *buf, const char *name) { - int i = 0, j = 0, dot = 0; + size_t i = 0, j = 0; + int dot = 0; if (buf[i] != '[') return 0; for (i = 1; buf[i] && buf[i] != ']'; i++) { @@ -3133,15 +3134,14 @@ static int git_config_copy_or_rename_section_in_file(const char *config_filename } while (!strbuf_getwholeline(&buf, config_file, '\n')) { - unsigned i; - int length; + size_t i, length; int is_section = 0; char *output = buf.buf; for (i = 0; buf.buf[i] && isspace(buf.buf[i]); i++) ; /* do nothing */ if (buf.buf[i] == '[') { /* it's a section */ - int offset; + size_t offset; is_section = 1; /* -- cgit v1.2.1