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

[Junio C Hamano <gitster@pobox.com>]

Han-Wen Nienhuys <hanwen@google.com> writes:

> +/*
> + * Optionally highlight one keyword in remote output if it appears at the start
> + * of the line. This should be called for a single line only, which is
> + * passed as the first N characters of the SRC array.
> + */
> +static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
> +{
> +	int i;
> +
> +	if (!want_color_stderr(use_sideband_colors())) {
> +		strbuf_add(dest, src, n);
> +		return;
> +	}
> +
> +	while (isspace(*src)) {
> +		strbuf_addch(dest, *src);
> +		src++;
> +		n--;
> +	}

This loop can run out of bytes in src in search of non-space before
n gets to zero or negative, and when that happens ...

> +	for (i = 0; i < ARRAY_SIZE(keywords); i++) {
> +		struct keyword_entry *p = keywords + i;
> +		int len = strlen(p->keyword);
> +		/*
> +		 * Match case insensitively, so we colorize output from existing
> +		 * servers regardless of the case that they use for their
> +		 * messages. We only highlight the word precisely, so
> +		 * "successful" stays uncolored.
> +		 */
> +		if (!strncasecmp(p->keyword, src, len) && !isalnum(src[len])) {

... these access src[] beyond the end of what the caller intended to
show us, and also ...

> +			strbuf_addstr(dest, p->color);
> +			strbuf_add(dest, src, len);
> +			strbuf_addstr(dest, GIT_COLOR_RESET);
> +			n -= len;
> +			src += len;
> +			break;
> +		}
> +	}
> +
> +	strbuf_add(dest, src, n);

... this will now try to add 0 or negative number of bytes.

> +
> +}
> +

Perhaps this will help (not really tested).  The second hunk is an
unrelated style clean-up.


 sideband.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/sideband.c b/sideband.c
index 1c6bb0e25b..d99a559a44 100644
--- a/sideband.c
+++ b/sideband.c
@@ -75,11 +75,13 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 		return;
 	}
 
-	while (isspace(*src)) {
+	while (isspace(*src) && n) {
 		strbuf_addch(dest, *src);
 		src++;
 		n--;
 	}
+	if (!n)
+		return;
 
 	for (i = 0; i < ARRAY_SIZE(keywords); i++) {
 		struct keyword_entry *p = keywords + i;
@@ -101,7 +103,6 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
 	}
 
 	strbuf_add(dest, src, n);
-
 }