Junio C Hamano wrote:
> Subject: sideband: do not read beyond the end of input
>
> The caller of maybe_colorize_sideband() gives a counted buffer
> <src,n>, but the callee checked *src as if it were a NUL terminated
> buffer. If src[] had all isspace() bytes in it, we would have made
> n negative, and then (1) called number of strncasecmp() to see if
> the remaining bytes in src[] matched keywords, reading beyond the
> end of the array, and/or (2) called strbuf_add() with negative
> count, most likely triggering the "you want to use way too much
> memory" error due to unsigned integer overflow.
>
> Signed-off-by: Junio C Hamano <[email protected]>
> ---
> sideband.c | 9 ++++++---
> 1 file changed, 6 insertions(+), 3 deletions(-)
And here are some tests to squash in.
A sideband line consisting entirely of spaces causes us to overflow
our buffer and end up with a negative number of remaining characters,
ultimately resulting in the message
fatal: you want to use way too much memory
when parsing it in order to add color.
We also forget to limit a strncasecmp and isalnum looking for keywords
to color in to the memory region passed in. Fortunately all callers
put a delimiter character (\0, \r, or \n) after the end of the region
so this does not cause trouble in practice. Add a test for
futureproofing to protect the new bounds checking code in case that
ever changes.
Signed-off-by: Jonathan Nieder <[email protected]>
---
Thanks again,
Jonathan
t/t5409-colorize-remote-messages.sh | 14 ++++++++++++++
1 file changed, 14 insertions(+)
diff --git a/t/t5409-colorize-remote-messages.sh
b/t/t5409-colorize-remote-messages.sh
index eb1b8aa05df..f81b6813c03 100755
--- a/t/t5409-colorize-remote-messages.sh
+++ b/t/t5409-colorize-remote-messages.sh
@@ -15,6 +15,8 @@ test_expect_success 'setup' '
echo warning: warning
echo prefixerror: error
echo " " "error: leading space"
+ echo " "
+ echo Err
exit 0
EOF
echo 1 >file &&
@@ -44,6 +46,12 @@ test_expect_success 'whole words at line start' '
grep "prefixerror: error" decoded
'
+test_expect_success 'short line' '
+ git -C child -c color.remote=always push -f origin HEAD:short-line
2>output &&
+ test_decode_color <output >decoded &&
+ grep "remote: Err" decoded
+'
+
test_expect_success 'case-insensitive' '
git --git-dir child/.git -c color.remote=always push -f origin
HEAD:refs/heads/case-insensitive 2>output &&
cat output &&
@@ -58,6 +66,12 @@ test_expect_success 'leading space' '
grep " <BOLD;RED>error<RESET>: leading space" decoded
'
+test_expect_success 'spaces only' '
+ git -C child -c color.remote=always push -f origin HEAD:only-space
2>output &&
+ test_decode_color <output >decoded &&
+ grep "remote: " decoded
+'
+
test_expect_success 'no coloring for redirected output' '
git --git-dir child/.git push -f origin
HEAD:refs/heads/redirected-output 2>output &&
test_decode_color <output >decoded &&
--
2.18.0.1017.ga543ac7ca45
