Making Cairo robust against big coordinates

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.

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.

<svg>
  <defs>
    <clipPath id="clipper">
      <rect y="19" width="2" height="2" />
      <rect width="18446744073709551616" height="29" />
    </clipPath>
  </defs>
  <g clip-path="url(#clipper)">
    <g clip-path="url(#clipper)">
    </g>
  </g>
</svg>

This generates this cairo-trace:

%!CairoScript - rsvg-view-3
/p0 0 0 0 0 rgba def
dict
  /width 1 set
  /height 1 set
  /format //RGB24 set
  /content //COLOR set
  image dup /s0 exch def
context % c0
save
save
save
identity set-matrix
//WINDING set-fill-rule
0 19 m 2 19 l 2 21 l 0 21 l 0 19 l h
0 19 m identity set-matrix
//WINDING set-fill-rule
0 0 m 18446744073709551616 0 l 18446744073709551616 29 l 0 29 l 0 0 l h
0 0 m clip
save
identity set-matrix
//WINDING set-fill-rule
0 19 m 2 19 l 2 21 l 0 21 l 0 19 l h
0 19 m identity set-matrix
//WINDING set-fill-rule
0 0 m 18446744073709551616 0 l 18446744073709551616 29 l 0 29 l 0 0 l h
0 0 m clip

If I run "valgrind rsvg-view-3 overflow.svg", I get

==3885== Use of uninitialised value of size 8
==3885==    at 0x6C2AF8D: insert_edge (cairo-boxes-intersect.c:458)
==3885==    by 0x6C2B04F: sweep_line_insert (cairo-boxes-intersect.c:478)
==3885==    by 0x6C2B1CD: intersect (cairo-boxes-intersect.c:516)
==3885==    by 0x6C2BDD0: _cairo_boxes_intersect (cairo-boxes-intersect.c:685)
==3885==    by 0x6C320A7: _cairo_clip_intersect_boxes (cairo-clip-boxes.c:290)
==3885==    by 0x6C31A99: _cairo_clip_intersect_rectilinear_path (cairo-clip-boxes.c:145)
==3885==    by 0x6C2FFF9: _cairo_clip_intersect_path (cairo-clip.c:261)
==3885==    by 0x6C3FDD3: _cairo_gstate_clip (cairo-gstate.c:1559)
==3885==    by 0x6C38E1C: _cairo_default_context_clip (cairo-default-context.c:1103)
==3885==    by 0x6C2DA24: cairo_clip (cairo.c:2492)
==3885==    by 0x4E68DD5: rsvg_cairo_clip (rsvg-cairo-clip.c:170)
==3885==    by 0x4E66E39: rsvg_cairo_push_early_clips (rsvg-cairo-draw.c:718)
==3885==
==3885== Invalid write of size 8
==3885==    at 0x6C2AF8D: insert_edge (cairo-boxes-intersect.c:458)
==3885==    by 0x6C2B04F: sweep_line_insert (cairo-boxes-intersect.c:478)
==3885==    by 0x6C2B1CD: intersect (cairo-boxes-intersect.c:516)
==3885==    by 0x6C2BDD0: _cairo_boxes_intersect (cairo-boxes-intersect.c:685)
==3885==    by 0x6C320A7: _cairo_clip_intersect_boxes (cairo-clip-boxes.c:290)
==3885==    by 0x6C31A99: _cairo_clip_intersect_rectilinear_path (cairo-clip-boxes.c:145)
==3885==    by 0x6C2FFF9: _cairo_clip_intersect_path (cairo-clip.c:261)
==3885==    by 0x6C3FDD3: _cairo_gstate_clip (cairo-gstate.c:1559)
==3885==    by 0x6C38E1C: _cairo_default_context_clip (cairo-default-context.c:1103)
==3885==    by 0x6C2DA24: cairo_clip (cairo.c:2492)
==3885==    by 0x4E68DD5: rsvg_cairo_clip (rsvg-cairo-clip.c:170)
==3885==    by 0x4E66E39: rsvg_cairo_push_early_clips (rsvg-cairo-draw.c:718)
==3885==  Address 0x9 is not stack'd, malloc'd or (recently) free'd

and then it crashes.

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 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
(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
#2  0x00007ffff5cab9d9 in _cairo_bentley_ottmann_tessellate_boxes (in=0x7fffffffd080, fill_rule=CAIRO_FILL_RULE_WINDING, out=0x7fffffffd080) at cairo-bentley-ottmann-rectangular.c:877
#3  0x00007ffff5ce550a in _cairo_path_fixed_fill_rectilinear_to_boxes (path=0x82c968, fill_rule=CAIRO_FILL_RULE_WINDING, antialias=CAIRO_ANTIALIAS_DEFAULT, boxes=0x7fffffffd080) at cairo-path-fill.c:333
#4  0x00007ffff5cbaa6e in _cairo_clip_intersect_rectilinear_path (clip=0x0, path=0x82c968, fill_rule=CAIRO_FILL_RULE_WINDING, antialias=CAIRO_ANTIALIAS_DEFAULT) at cairo-clip-boxes.c:140
#5  0x00007ffff5cb8ffa in _cairo_clip_intersect_path (clip=0x0, path=0x82c968, fill_rule=CAIRO_FILL_RULE_WINDING, tolerance=0.10000000000000001, antialias=CAIRO_ANTIALIAS_DEFAULT) at cairo-clip.c:261
#6  0x00007ffff5cc8e2c in _cairo_gstate_clip (gstate=0x713160, path=0x82c968) at cairo-gstate.c:1559
#7  0x00007ffff5cc1e49 in _cairo_default_context_clip (abstract_cr=0x82c600) at cairo-default-context.c:1103
#8  0x00007ffff5cb6a25 in cairo_clip (cr=0x82c600) at cairo.c:2492
#9  0x00007ffff7bd1dd6 in rsvg_cairo_clip (ctx=0x6bfe40, clip=0x6d43a0, bbox=0x0) at rsvg-cairo-clip.c:170
#10 0x00007ffff7bcfe3a in rsvg_cairo_push_early_clips (ctx=0x6bfe40) at rsvg-cairo-draw.c:718
#11 0x00007ffff7bd00e4 in rsvg_cairo_push_discrete_layer (ctx=0x6bfe40) at rsvg-cairo-draw.c:777
#12 0x00007ffff7bcc1f6 in rsvg_push_discrete_layer (ctx=0x6bfe40) at rsvg-base.c:1997
#13 0x00007ffff7bbf4af in _rsvg_node_draw_children (self=0x700fc0, ctx=0x6bfe40, dominate=0) at rsvg-structure.c:83
#14 0x00007ffff7bbf444 in rsvg_node_draw (self=0x700fc0, ctx=0x6bfe40, dominate=0) at rsvg-structure.c:69
#15 0x00007ffff7bc0287 in rsvg_node_svg_draw (self=0x712eb0, ctx=0x6bfe40, dominate=0) at rsvg-structure.c:323
#16 0x00007ffff7bbf444 in rsvg_node_draw (self=0x712eb0, ctx=0x6bfe40, dominate=0) at rsvg-structure.c:69
#17 0x00007ffff7bcad15 in rsvg_handle_get_dimensions_sub (handle=0x7b90d0, dimension_data=0x7fffffffdae8, id=0x0) at rsvg-base.c:1404
#18 0x00007ffff7bca9bc in rsvg_handle_get_dimensions (handle=0x7b90d0, dimension_data=0x7fffffffdae8) at rsvg-base.c:1317
#19 0x0000000000405634 in main (argc=1, argv=0x7fffffffdcc8) at test-display.c:773

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?

  Federico

-- 
GPG fingerprint - RSA 4096:
263F 590F 7E0F E1CB 3EA2  74B0 1676 37EB 6FB8 DCCE

-- 
cairo mailing list
[email protected]
http://lists.cairographics.org/mailman/listinfo/cairo

diff --git a/src/cairo-fixed-private.h b/src/cairo-fixed-private.h
index 9ff8f75..07d0ced 100644
--- a/src/cairo-fixed-private.h
+++ b/src/cairo-fixed-private.h
@@ -106,6 +106,9 @@ _cairo_fixed_from_int (int i)
 #if CAIRO_FIXED_BITS <= 32
 #define CAIRO_MAGIC_NUMBER_FIXED ((1LL << (52 - CAIRO_FIXED_FRAC_BITS)) * 1.5)
 
+#define CAIRO_FIXED_MAX_DOUBLE ((double) CAIRO_FIXED_MAX_VALUE / CAIRO_FIXED_ONE_DOUBLE)
+#define CAIRO_FIXED_MIN_DOUBLE ((double) CAIRO_FIXED_MIN_VALUE / CAIRO_FIXED_ONE_DOUBLE)
+
 /* For 32-bit fixed point numbers */
 static inline cairo_fixed_t
 _cairo_fixed_from_double (double d)
@@ -115,6 +118,11 @@ _cairo_fixed_from_double (double d)
         int32_t i[2];
     } u;
 
+    if (unlikely (d > CAIRO_FIXED_MAX_DOUBLE))
+	return CAIRO_FIXED_MAX_VALUE;
+    else if (unlikely (d < CAIRO_FIXED_MIN_DOUBLE))
+	return CAIRO_FIXED_MIN_VALUE;
+
     u.d = d + CAIRO_MAGIC_NUMBER_FIXED;
 #ifdef FLOAT_WORDS_BIGENDIAN
     return u.i[1];
diff --git a/src/cairo-fixed-type-private.h b/src/cairo-fixed-type-private.h
index e9f26f6..74ab24b 100644
--- a/src/cairo-fixed-type-private.h
+++ b/src/cairo-fixed-type-private.h
@@ -64,6 +64,9 @@ typedef cairo_int128_t	cairo_fixed_96_32_t;
 /* A signed type %CAIRO_FIXED_BITS in size; the main fixed point type */
 typedef int32_t cairo_fixed_t;
 
+#define CAIRO_FIXED_MAX_VALUE ((cairo_fixed_t) 0x7fffffff)
+#define CAIRO_FIXED_MIN_VALUE ((cairo_fixed_t) (-CAIRO_FIXED_MAX_VALUE - 1))
+
 /* An unsigned type of the same size as #cairo_fixed_t */
 typedef uint32_t cairo_fixed_unsigned_t;