[Pixman] [PATCH] pixman-general: Fix stack related pointer arithmetic overflow

[Oded Gabbay]

On Tue, Sep 22, 2015 at 4:25 AM, Siarhei Siamashka
<siarhei.siamashka at gmail.com> wrote:
> As https://bugs.freedesktop.org/show_bug.cgi?id=92027#c6 explains,
> the stack is allocated at the very top of the process address space
> in some configurations (32-bit x86 systems with ASLR disabled).
> And the careless computations done with the 'dest_buffer' pointer
> may overflow, failing the buffer upper limit check.
>
> The problem can be reproduced using the 'stress-test' program,
> which segfaults when executed via setarch:
>
>     export CFLAGS="-O2 -m32" && ./autogen.sh
>     ./configure --disable-libpng --disable-gtk && make
>     setarch i686 -R test/stress-test
>
> This patch introduces the required corrections. The extra check
> for negative 'width' may be redundant (the invalid 'width' value
> is not supposed to reach here), but it's better to play safe
> when dealing with the buffers allocated on stack.
>
> Reported-by: Ludovic Courtès <ludo at gnu.org>
> Signed-off-by: Siarhei Siamashka <siarhei.siamashka at gmail.com>
> ---
>  pixman/pixman-general.c | 16 +++++++---------
>  1 file changed, 7 insertions(+), 9 deletions(-)
>
> diff --git a/pixman/pixman-general.c b/pixman/pixman-general.c
> index 7cdea29..fa88463 100644
> --- a/pixman/pixman-general.c
> +++ b/pixman/pixman-general.c
> @@ -155,23 +155,21 @@ general_composite_rect  (pixman_implementation_t *imp,
>  #define ALIGN(addr)                                                    \
>      ((uint8_t *)((((uintptr_t)(addr)) + 15) & (~15)))
>
> -    src_buffer = ALIGN (scanline_buffer);
> -    mask_buffer = ALIGN (src_buffer + width * Bpp);
> -    dest_buffer = ALIGN (mask_buffer + width * Bpp);
> +    if (width <= 0 || _pixman_multiply_overflows_int (width, Bpp * 3))
> +       return;
>
> -    if (ALIGN (dest_buffer + width * Bpp) >
> -           scanline_buffer + sizeof (stack_scanline_buffer))
> +    if (width * Bpp * 3 > sizeof (stack_scanline_buffer) - 32 * 3)
>      {
>         scanline_buffer = pixman_malloc_ab_plus_c (width, Bpp * 3, 32 * 3);
>
>         if (!scanline_buffer)
>             return;
> -
> -       src_buffer = ALIGN (scanline_buffer);
> -       mask_buffer = ALIGN (src_buffer + width * Bpp);
> -       dest_buffer = ALIGN (mask_buffer + width * Bpp);
>      }
>
> +    src_buffer = ALIGN (scanline_buffer);
> +    mask_buffer = ALIGN (src_buffer + width * Bpp);
> +    dest_buffer = ALIGN (mask_buffer + width * Bpp);
> +
>      if (width_flag == ITER_WIDE)
>      {
>         /* To make sure there aren't any NANs in the buffers */
> --
> 2.4.6
>

I tested the patch and it fixed the crash on my computer.

I pushed it to master:
4297e90..8b49d4b  master -> master

and to 0.32 branch:
d6a4a56..204fcd2  0.32 -> 0.32

       Oded