@matheusbsbs / matheus-bagliano / commits / 48050c42c7

pretty: fix integer overflow in wrapping format

The `%w(width,indent1,indent2)` formatting directive can be used to rewrap text to a specific width and is designed after git-shortlog(1)'s `-w` parameter. While the three parameters are all stored as `size_t` internally, `strbuf_add_wrapped_text()` accepts integers as input. As a result, the casted integers may overflow. As these now-negative integers are later on passed to `strbuf_addchars()`, we will ultimately run into implementation-defined behaviour due to casting a negative number back to `size_t` again. On my platform, this results in trying to allocate 9000 petabyte of memory. Fix this overflow by using `cast_size_t_to_int()` so that we reject inputs that cannot be represented as an integer. Signed-off-by: Patrick Steinhardt <ps@pks.im> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Patrick Steinhardt committed Dec 1, 2022 at 15:46 UTC 48050c42c73c28b0c001d63d11dffac7e116847b

3 files changed +23 -1

git-compat-util.h

index f505f817d5..0ac1b7f560 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -918,6 +918,14 @@ static inline size_t st_sub(size_t a, size_t b) return a - b; } +static inline int cast_size_t_to_int(size_t a) +{ + if (a > INT_MAX) + die("number too large to represent as int on this platform: %"PRIuMAX, + (uintmax_t)a); + return (int)a; +} + #ifdef HAVE_ALLOCA_H # include <alloca.h> # define xalloca(size) (alloca(size))

pretty.c

+3 -1

index c6c757c0ce..7e649b1cec 100644 --- a/pretty.c +++ b/pretty.c @@ -915,7 +915,9 @@ static void strbuf_wrap(struct strbuf *sb, size_t pos, if (pos) strbuf_add(&tmp, sb->buf, pos); strbuf_add_wrapped_text(&tmp, sb->buf + pos, - (int) indent1, (int) indent2, (int) width); + cast_size_t_to_int(indent1), + cast_size_t_to_int(indent2), + cast_size_t_to_int(width)); strbuf_swap(&tmp, sb); strbuf_release(&tmp); }

t/t4205-log-pretty-formats.sh

+12

index 1d768f7244..c88b64d08b 100755 --- a/t/t4205-log-pretty-formats.sh +++ b/t/t4205-log-pretty-formats.sh @@ -887,6 +887,18 @@ test_expect_success 'log --pretty with magical wrapping directives' ' test_cmp expect actual ' +test_expect_success SIZE_T_IS_64BIT 'log --pretty with overflowing wrapping directive' ' + cat >expect <<-EOF && + fatal: number too large to represent as int on this platform: 2147483649 + EOF + test_must_fail git log -1 --pretty="format:%w(2147483649,1,1)%d" 2>error && + test_cmp expect error && + test_must_fail git log -1 --pretty="format:%w(1,2147483649,1)%d" 2>error && + test_cmp expect error && + test_must_fail git log -1 --pretty="format:%w(1,1,2147483649)%d" 2>error && + test_cmp expect error +' + test_expect_success EXPENSIVE,SIZE_T_IS_64BIT 'log --pretty with huge commit message' ' # We only assert that this command does not crash. This needs to be # executed with the address sanitizer to demonstrate failure.