pretty: restrict input lengths for padding and wrapping formats

Both the padding and wrapping formatting directives allow the caller to specify an integer that ultimately leads to us adding this many chars to the result buffer. As a consequence, it is trivial to e.g. allocate 2GB of RAM via a single formatting directive and cause resource exhaustion on the machine executing this logic. Furthermore, it is debatable whether there are any sane usecases that require the user to pad data to 2GB boundaries or to indent wrapped data by 2GB. Restrict the input sizes to 16 kilobytes at a maximum to limit the amount of bytes that can be requested by the user. This is not meant as a fix because there are ways to trivially amplify the amount of data we generate via formatting directives; the real protection is achieved by the changes in previous steps to catch and avoid integer wraparound that causes us to under-allocate and access beyond the end of allocated memory reagions. But having such a limit significantly helps fuzzing the pretty format, because the fuzzer is otherwise quite fast to run out-of-memory as it discovers these formatters. 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:47 UTC 304a50adff6480ede46b68f7545baab542cbfb46
2 files changed +41 -9
pretty.c
+26
@@ -13,6 +13,13 @@
13 #include "gpg-interface.h"
14 #include "trailer.h"
15
16 +/*
17 + * The limit for formatting directives, which enable the caller to append
18 + * arbitrarily many bytes to the formatted buffer. This includes padding
19 + * and wrapping formatters.
20 + */
21 +#define FORMATTING_LIMIT (16 * 1024)
22 +
23 static char *user_format;
24 static struct cmt_fmt_map {
25 const char *name;
@@ -1046,6 +1053,15 @@ static size_t parse_padding_placeholder(const char *placeholder,
1053 if (!*end || end == start)
1054 return 0;
1055 width = strtol(start, &next, 10);
1056 +
1057 + /*
1058 + * We need to limit the amount of padding, or otherwise this
1059 + * would allow the user to pad the buffer by arbitrarily many
1060 + * bytes and thus cause resource exhaustion.
1061 + */
1062 + if (width < -FORMATTING_LIMIT || width > FORMATTING_LIMIT)
1063 + return 0;
1064 +
1065 if (next == start || width == 0)
1066 return 0;
1067 if (width < 0) {
@@ -1205,6 +1221,16 @@ static size_t format_commit_one(struct strbuf *sb, /* in UTF-8 */
1221 if (*next != ')')
1222 return 0;
1223 }
1224 +
1225 + /*
1226 + * We need to limit the format here as it allows the
1227 + * user to prepend arbitrarily many bytes to the buffer
1228 + * when rewrapping.
1229 + */
1230 + if (width > FORMATTING_LIMIT ||
1231 + indent1 > FORMATTING_LIMIT ||
1232 + indent2 > FORMATTING_LIMIT)
1233 + return 0;
1234 rewrap_message_tail(sb, c, width, indent1, indent2);
1235 return end - placeholder + 1;
1236 } else
t/t4205-log-pretty-formats.sh
+15 -9
@@ -888,15 +888,21 @@ test_expect_success 'log --pretty with magical wrapping directives' '
888 '
889
890 test_expect_success SIZE_T_IS_64BIT 'log --pretty with overflowing wrapping directive' '
891 - cat >expect <<-EOF &&
892 - fatal: number too large to represent as int on this platform: 2147483649
893 - EOF
894 - test_must_fail git log -1 --pretty="format:%w(2147483649,1,1)%d" 2>error &&
895 - test_cmp expect error &&
896 - test_must_fail git log -1 --pretty="format:%w(1,2147483649,1)%d" 2>error &&
897 - test_cmp expect error &&
898 - test_must_fail git log -1 --pretty="format:%w(1,1,2147483649)%d" 2>error &&
899 - test_cmp expect error
891 + printf "%%w(2147483649,1,1)0" >expect &&
892 + git log -1 --pretty="format:%w(2147483649,1,1)%x30" >actual &&
893 + test_cmp expect actual &&
894 + printf "%%w(1,2147483649,1)0" >expect &&
895 + git log -1 --pretty="format:%w(1,2147483649,1)%x30" >actual &&
896 + test_cmp expect actual &&
897 + printf "%%w(1,1,2147483649)0" >expect &&
898 + git log -1 --pretty="format:%w(1,1,2147483649)%x30" >actual &&
899 + test_cmp expect actual
900 +'
901 +
902 +test_expect_success SIZE_T_IS_64BIT 'log --pretty with overflowing padding directive' '
903 + printf "%%<(2147483649)0" >expect &&
904 + git log -1 --pretty="format:%<(2147483649)%x30" >actual &&
905 + test_cmp expect actual
906 '
907
908 test_expect_success 'log --pretty with padding and preceding control chars' '