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

> > Actually, let's just lose the conditional.  strbuf_add() would catch
> > and issue an error message when it notices that we fed negative
> > count anyway ;-).
>
> So I'll have this applied on top of the original topic to prevent a
> buggy version from escaping the lab.
>
> -- >8 --
> Subject: [PATCH] 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) made number of strncasecmp() calls to see if the remaining
>      bytes in src[] matched keywords, reading beyond the end of the
>      array (this actually happens even if n does not go negative),
>      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.
>
> Fix both issues by making sure we do not go beyond &src[n].
>
> In the longer term we may want to accept size_t as parameter for
> clarity (even though we know that a sideband message we are painting
> typically would fit on a line on a terminal and int is sufficient).
> Write it down as a NEEDSWORK comment.
>
> Helped-by: Jonathan Nieder <jrnieder@gmail.com>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
>  sideband.c                          |  8 ++++++--
>  t/t5409-colorize-remote-messages.sh | 14 ++++++++++++++
>  2 files changed, 20 insertions(+), 2 deletions(-)
>
> diff --git a/sideband.c b/sideband.c
> index 1c6bb0e25b..368647acf8 100644
> --- a/sideband.c
> +++ b/sideband.c
> @@ -65,6 +65,8 @@ void list_config_color_sideband_slots(struct string_list *list, const char *pref
>   * 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.
> + *
> + * NEEDSWORK: use "size_t n" instead for clarity.
>   */
>  static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
>  {
> @@ -75,7 +77,7 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
>                 return;
>         }
>
> -       while (isspace(*src)) {
> +       while (0 < n && isspace(*src)) {
>                 strbuf_addch(dest, *src);
>                 src++;
>                 n--;
> @@ -84,6 +86,9 @@ static void maybe_colorize_sideband(struct strbuf *dest, const char *src, int n)
>         for (i = 0; i < ARRAY_SIZE(keywords); i++) {
>                 struct keyword_entry *p = keywords + i;
>                 int len = strlen(p->keyword);
> +
> +               if (n <= len)
> +                       continue;