Re: Re* [PATCH v7 1/1] sideband: highlight keywords in remote sideband output

[Jonathan Nieder <jrnieder@gmail.com>]

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 <gitster@pobox.com>
> ---
>  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 <jrnieder@gmail.com>
---
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