[PATCH] less: fix botched attempt to use last column

[Denys Vlasenko]

Applied, thanks!

On Tue, Aug 4, 2015 at 6:10 PM, Ron Yorston <rmy at pobox.com> wrote:
> Commit 1ecb996 attempted to make read_lines() use the last column of
> the terminal (as re_wrap() did).  There were two problems with this:
>
> - The size of the buffer allocated for lines wasn't increased to allow
>   for the extra character.
>
> - The test for width overflow was moved after the point where the
>   next character was added to the buffer.  This caused a buffer overflow
>   in certain circumstances.
>
>   For example, if the line beyond the end of the display was wider than
>   the display read_lines() would initially read the partial line into a
>   buffer.  When the user moved down read_lines() would be called again
>   to ensure the rest of the line was read.  This would place the next
>   character in the partial line before checking for overflow.
>
>   This can be fixed by moving the test for overflow back to where it was
>   before commit 1ecb996 and changing the comparison to `>` rather than
>   `>=`.
>
> There are two other places where buffers are created without allowing
> for width+1 characters.
>
> Signed-off-by: Ron Yorston <rmy at pobox.com>
> ---
>  miscutils/less.c | 28 ++++++++++++++--------------
>  1 file changed, 14 insertions(+), 14 deletions(-)
>
> diff --git a/miscutils/less.c b/miscutils/less.c
> index 7a441bf..ccdb15f 100644
> --- a/miscutils/less.c
> +++ b/miscutils/less.c
> @@ -456,7 +456,7 @@ static void read_lines(void)
>         if (option_mask32 & FLAG_N)
>                 w -= 8;
>
> -       p = current_line = ((char*)xmalloc(w + 4)) + 4;
> +       p = current_line = ((char*)xmalloc(w + 5)) + 4;
>         if (!last_terminated) {
>                 const char *cp = flines[max_fline];
>                 p = stpcpy(p, cp);
> @@ -509,6 +509,16 @@ static void read_lines(void)
>                                 *--p = '\0';
>                                 continue;
>                         }
> +                       {
> +                               size_t new_last_line_pos = last_line_pos + 1;
> +                               if (c == '\t') {
> +                                       new_last_line_pos += 7;
> +                                       new_last_line_pos &= (~7);
> +                               }
> +                               if ((int)new_last_line_pos > w)
> +                                       break;
> +                               last_line_pos = new_last_line_pos;
> +                       }
>                         /* ok, we will eat this char */
>                         readpos++;
>                         if (c == '\n') {
> @@ -520,16 +530,6 @@ static void read_lines(void)
>                         if (c == '\0') c = '\n';
>                         *p++ = c;
>                         *p = '\0';
> -                       {
> -                               size_t new_last_line_pos = last_line_pos + 1;
> -                               if (c == '\t') {
> -                                       new_last_line_pos += 7;
> -                                       new_last_line_pos &= (~7);
> -                               }
> -                               if ((int)new_last_line_pos >= w)
> -                                       break;
> -                               last_line_pos = new_last_line_pos;
> -                       }
>                 } /* end of "read chars until we have a line" loop */
>  #if 0
>  //BUG: also triggers on this:
> @@ -573,7 +573,7 @@ static void read_lines(void)
>                         break;
>                 }
>                 max_fline++;
> -               current_line = ((char*)xmalloc(w + 4)) + 4;
> +               current_line = ((char*)xmalloc(w + 5)) + 4;
>                 p = current_line;
>                 last_line_pos = 0;
>         } /* end of "read lines until we reach cur_fline" loop */
> @@ -755,7 +755,7 @@ static void print_found(const char *line)
>         char *growline;
>         regmatch_t match_structs;
>
> -       char buf[width];
> +       char buf[width+1];
>         const char *str = line;
>         char *p = buf;
>         size_t n;
> @@ -814,7 +814,7 @@ void print_found(const char *line);
>
>  static void print_ascii(const char *str)
>  {
> -       char buf[width];
> +       char buf[width+1];
>         char *p;
>         size_t n;
>
> --
> 2.4.3
>
> _______________________________________________
> busybox mailing list
> busybox at busybox.net
> http://lists.busybox.net/mailman/listinfo/busybox