[prev in list] [next in list] [prev in thread] [next in thread]
List: cairo
Subject: [cairo] Making Cairo robust against big coordinates
From: Federico Mena Quintero <federico.mena.lists () gmail ! com>
Date: 2015-02-24 1:32:50
Message-ID: 1424741570.3001.11.camel () tlacoyo
[Download RAW message or body]
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-fixed-from-double-overflow.patch" (cairo-fixed-from-double-overflow.patch)]
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;
[Attachment #4 (text/plain)]
--
cairo mailing list
cairo@cairographics.org
http://lists.cairographics.org/mailman/listinfo/cairo
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic