[cairo-bugs] [Bug 98165] DoS attack based on using SVG to generate invalid pointers from a _cairo_image_surface in write_png

[bugzilla-daemon at freedesktop.org]

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

--- Comment #3 from John Bowler <jbowler at acm.org> ---
Well, yes, stride should be (size_t), but there may be other instances of this.

If you change the type of stride in the struct to (unsigned int), from (int)
and run with the correct compiler warning options it will warn about:

    (int) * (unsigned int)

because the (int) gets converted silently to (unsigned int).  GCC probably
ignores this by default, but the -Wconversion stuff is meant to detect it. 
Coverity certainly can.

Doing the above temporarily will tell you if any other code in libcairo does
this.  It doesn't catch all the potential problems; for example read_png
already has 'i' as (unsigned int) and does (IRC):

    i * stride

That still overflows on a 64-bit system, it just requires a bigger SVG and it
is a 'safe' overflow because all the pointers are still inside the image
buffer.

This is why I suggested changing the struct member; it is difficult to detect
potential 32-bit overflow.  I don't think even Coverity warns about 32-bit
arithmetic being used inside a 64-bit address calculation and it is extremely
common and normally safe.

The other approach you could use is to check when the cairo surface is created
to make sure it doesn't require more than a 31, or 32-bit sized buffer. 
However there are some devices out there which can exceed a 4GByte image; think
of a 72" poster printer running at 1200dpi.  That has 86400 dots (bytes) per
row so a 42" high printout would exceed the limit.

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