[cairo] Making Cairo robust against big coordinates

[Bryce Harrington]

On Mon, Feb 23, 2015 at 07:32:50PM -0600, Federico Mena Quintero wrote:
> Hi, everyone,
> 
> I'm fixing bugs in librsvg that have been exposed through fuzz-testing,
> and some of them turn out to affect Cairo.
> 
> The first problem is that Cairo doesn't try to deal with "big"
> coordinates, those that would overflow its internal fixed-point
> representation.  This is not just "garbage rendering"; Cairo actually
> crashes.

Yeah, Cairo could probably benefit from similar fuzz tests.
 
> I'm pasting a little SVG that crashes Cairo if you run, for example,
> "rsvg-view-3 overflow.svg".  I've not made this an attachment to this
> mail, as (at least) Evolution tries to thumbnail the SVG and crashes.
> 
> and then it crashes.

Hah!

> I started to play with modifying _cairo_fixed_from_double(), to make it
> clamp values to the maximum range that the fixed-point representation
> supports; a patch is attached.

This patch looks sane.  I'm actually a bit surprised this function isn't
handling floating point bounds better than it is.  Even though it
doesn't solve the original problem, this looks like it would address
flaws in the function implementation.  If you provide a cairo test case
to go along with this, I think it can be landed in trunk (and maybe on
stable).

> This changes the crash to
> 
> Program received signal SIGSEGV, Segmentation fault.
> 0x00007ffff5caa63b in active_edges_to_traps (sweep=0x7fffffffbd10) at cairo-bentley-ottmann-rectangular.c:506
> 506                     if (winding == 0 && right->x != right->next->x)
> (gdb) p right
> $41 = (edge_t *) 0x7fffffffbd48

Perhaps this needs a check for right->next being non-null?
Would be interesting to see a dump of the sweep and right structures.

> (gdb) where
> #0  0x00007ffff5caa63b in active_edges_to_traps (sweep=0x7fffffffbd10) at cairo-bentley-ottmann-rectangular.c:506
> #1  0x00007ffff5caaa8b in _cairo_bentley_ottmann_tessellate_rectangular (rectangles=0x7fffffffc6e0, num_rectangles=2, fill_rule=CAIRO_FILL_RULE_WINDING, do_traps=0, container=0x7fffffffd080) at cairo-bentley-ottmann-rectangular.c:636
> 
> I haven't read active_edges_to_traps() yet to see what it is doing;
> maybe some integer overflow is causing it to try to look in the wrong
> node.
> 
> What do people think?

_cairo_bentley_ottmann_tessellate_rectangular has two do..while loop
structures, and this is crashing in the second one.  The second one
looks like it's handling some end condition that presumably wasn't
covered in the first.  But this chunk of code we're passing through in
b.o.tessellate_rectangular doesn't appear to have changed much since it
was introduced in commit b83f1c34 from 2010.

active_edges_to_traps is more interesting.  The particular line that's
crashing is attempting to skip co-linear edges.  There's been several
changes to the function since its introduction, such as c09be6811
(fixing fdo bug #49446), ee001b0b, 014e5e5e, ...

Anyway, easist thing to test first is that right->next pointer.
Can you check if the patch below resolves the crash?  (FTR, I don't
propose this as a fix; if it resolves the crash we need to understand
why it's getting a NULL next pointer...)

Bryce

diff --git a/src/cairo-bentley-ottmann-rectangular.c b/src/cairo-bentley-ottmann-rectangular.c
index 5541bdc..51c8893 100644
--- a/src/cairo-bentley-ottmann-rectangular.c
+++ b/src/cairo-bentley-ottmann-rectangular.c
@@ -507,7 +507,7 @@ active_edges_to_traps (sweep_line_t *sweep)
 		    break;
 
 		right = right->next;
-	    } while (TRUE);
+	    } while (right->next != NULL);
 
 	    edge_start_or_continue_box (sweep, left, right, top);