[cairo] Crash bug in cairo-quartz-surface.c

[Baz]

On Dec 6, 2007 12:11 AM, Boris Zbarsky <bzbarsky at mit.edu> wrote:
> I just ran into a crash in ComputeGradientValue in cairo-quartz-surface.c.  In
> particular, the caller passes in an empty gradient (grad->n_stops == 0).

Yep, I've got a test reproducing that now.

> The relevant part of the code looks like this:
>
>    for (i = 0; i < grad->n_stops; i++) {
>      if (grad->stops[i].x > fdist_fix)
>        break;
>    }
>
>    if (i == 0 || i == grad->n_stops) {
>      if (i == grad->n_stops)
>        --i;
>
>      out[0] = grad->stops[i].color.red;
>
> When n_stops is 0, we end up with i == 0 after the first loop, then i ==
> grad->n_stops, so we decrement i.  i is declared as unsigned int, so we end up
> accessing grad->stops[0xffffffff] and crash.  Of course if it were a signed int
> we'd be accessing grad->stops[-1] which isn't so much better....
>
> I'm not sure what this function should be doing if it gets a gradient with no
> stops or whether it should just not be called at all...

Not called at all... the correct behaviour here is a solid alpha fill
(at least, that's what the image surface does).

I've got a fix for this. Slightly more than a one liner, I needed to
refactor _cairo_quartz_cairo_gradient_pattern_to_quartz into new
functions _cairo_quartz_setup_linear_source and
_cairo_quartz_setup_radial_source, after that the fix is easy. The
split is something that was going to be needed anyway if I get round
to doing the missing EXTEND_* cases for radial and linear gradients.

I'll probably push this tomorrow, the fix is fine but the test needs
tidied up a bit.

>
>    ComputeGradientValue
>    CGFunctionEvaluate
>    drawAxialShading
>    CGContextDrawShading
>    _cairo_quartz_surface_fill
>
> -Boris

If there's a mozilla bug CC me in please, I'll follow up there.

Cheers,
Baz

> _______________________________________________
> cairo mailing list
> cairo at cairographics.org
> http://lists.cairographics.org/mailman/listinfo/cairo
>