[cairo-bugs] [Bug 90120] Image compositor can pass invalid coordinates to pixman_fill()

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=90120

--- Comment #6 from Bryce Harrington <b.harrington at samsung.com> ---
If I understand this particular bug correctly, the issue is that librsvg can
pass in stroke widths that exceed the practical limits that cairo can handle.

Checking the parameters before making the pixman call sounds like it addresses
the crash, but has two problems - first, since this bit of code is pretty low
level there may be performance issues; second, as Chris said the problem starts
higher up so this may paper over where the garbage data causes one crash, but
leaves the garbage data in there to cause other problems that might be less
obvious than crashes (e.g. mis-rendering).

Looking higher up the stack, _cairo_surface_fill_stroke may be a better place
to do the checking.  It already is validating surfaces and such, so would make
sense for it to check for this condition too.  I'm not sure what that code
would look like, though.

But we may be able to address it even earlier; the garbage data I assume comes
from the excessively large line stroke width generated by the fuzz tester and
thus I assume is getting set via a call to cairo_set_line_width():

rsvg_cairo_render_path (RsvgDrawingCtx * ctx, const cairo_path_t *path)
{
    ...
    cairo_set_line_width (cr, _rsvg_css_normalize_length (&state->stroke_width,
ctx, 'h'));  // <-- garbage-in?
    cairo_set_miter_limit (cr, state->miter_limit);
    cairo_set_line_cap (cr, (cairo_line_cap_t) state->cap);
    cairo_set_line_join (cr, (cairo_line_join_t) state->join);
    cairo_set_dash (cr, state->dash.dash, state->dash.n_dash,
                    _rsvg_css_normalize_length (&state->dash.offset, ctx,
'o'));
    ...
    cairo_stroke (cr); // !CRASH!

cairo_set_line_width() already does some input checking by clamping underrun
widths to 0.  This could be made to similarly clamp maximum values, or flag an
error via _cairo_set_error.  It's unclear what that maximum should be though;
something smaller than 4294967294 presumably.

Even if we add a check to cairo_set_line_width(), it is possible that scaling
or other operations subsequent to that could modify things to invalid sizes. 
So it makes sense to also check in _cairo_surface_fill_stroke.

-- 
You are receiving this mail because:
You are the QA Contact for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.cairographics.org/archives/cairo-bugs/attachments/20150421/7ab937fb/attachment.html>