brl/mutter
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

path: Avoid integer overflow in get_distance()

Browse Source 
The get_distance() API uses machine integers to compute the distance;
this means that on 32bit we can overflow the integer size. This gets
hidden by the fact that get_distance() returns an unsigned integer as
well.

In reality, ClutterPath is an unmitigated mess, and the only way to
actually fix it is to break API.

https://bugzilla.gnome.org/show_bug.cgi?id=652521
This commit is contained in:
Emmanuele Bassi 2012-04-23 17:42:40 +01:00
parent 9428aaa3cb
commit 0fca11ec2f
2 changed files with 53 additions and 20 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

41
clutter/clutter-path.c
View File

@ -632,14 +632,14 @@ clutter_path_parse_description (const gchar *p,
node = clutter_path_node_full_new (); node = clutter_path_node_full_new ();
nodes = g_slist_prepend (nodes, node); nodes = g_slist_prepend (nodes, node);
node->k.type = (*p == 'M' ? CLUTTER_PATH_MOVE_TO node->k.type = (*p == 'M' ? CLUTTER_PATH_MOVE_TO :
: *p == 'm' ? CLUTTER_PATH_REL_MOVE_TO *p == 'm' ? CLUTTER_PATH_REL_MOVE_TO :
: *p == 'L' ? CLUTTER_PATH_LINE_TO *p == 'L' ? CLUTTER_PATH_LINE_TO :
: CLUTTER_PATH_REL_LINE_TO); CLUTTER_PATH_REL_LINE_TO);
p++; p++;
if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x) if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y)) !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y))
goto fail; goto fail;
break; break;
@ -648,16 +648,16 @@ clutter_path_parse_description (const gchar *p,
node = clutter_path_node_full_new (); node = clutter_path_node_full_new ();
nodes = g_slist_prepend (nodes, node); nodes = g_slist_prepend (nodes, node);
node->k.type = (*p == 'C' ? CLUTTER_PATH_CURVE_TO node->k.type = (*p == 'C' ? CLUTTER_PATH_CURVE_TO :
: CLUTTER_PATH_REL_CURVE_TO); CLUTTER_PATH_REL_CURVE_TO);
p++; p++;
if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x) if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y) !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[1].x) !clutter_path_parse_number (&p, TRUE, &node->k.points[1].x) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[1].y) !clutter_path_parse_number (&p, TRUE, &node->k.points[1].y) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[2].x) !clutter_path_parse_number (&p, TRUE, &node->k.points[2].x) ||
|| !clutter_path_parse_number (&p, TRUE, &node->k.points[2].y)) !clutter_path_parse_number (&p, TRUE, &node->k.points[2].y))
goto fail; goto fail;
break; break;
@ -1244,11 +1244,12 @@ clutter_path_get_description (ClutterPath *path)
return g_string_free (str, FALSE); return g_string_free (str, FALSE);
} }
static gint static guint
clutter_path_node_distance (const ClutterKnot *start, clutter_path_node_distance (const ClutterKnot *start,
const ClutterKnot *end) const ClutterKnot *end)
{ {
gint t; gint64 x_d, y_d;
float t;
g_return_val_if_fail (start != NULL, 0); g_return_val_if_fail (start != NULL, 0);
g_return_val_if_fail (end != NULL, 0); g_return_val_if_fail (end != NULL, 0);
@ -1256,10 +1257,12 @@ clutter_path_node_distance (const ClutterKnot *start,
if (clutter_knot_equal (start, end)) if (clutter_knot_equal (start, end))
return 0; return 0;
t = (end->x - start->x) * (end->x - start->x) + x_d = end->x - start->x;
(end->y - start->y) * (end->y - start->y); y_d = end->y - start->y;
return sqrtf (t); t = floorf (sqrtf ((x_d * x_d) + (y_d * y_d)));
return (guint) t;
} }
static void static void

32
tests/conform/path.c
View File

@ -551,12 +551,42 @@ path_test_get_length (CallbackData *data)
const float actual_length /* sqrt(64**2 + 64**2) * 2 */ = 181.019336f; const float actual_length /* sqrt(64**2 + 64**2) * 2 */ = 181.019336f;
guint approx_length; guint approx_length;
clutter_path_set_description (data->path, "M 0 0 L 46340 0");
g_object_get (data->path, "length", &approx_length, NULL);
if (!(fabs (approx_length - 46340.f) / 46340.f <= 0.15f))
{
if (g_test_verbose ())
g_print ("M 0 0 L 46340 0 - Expected 46340, got %d instead.", approx_length);
return FALSE;
}
clutter_path_set_description (data->path, "M 0 0 L 46341 0");
g_object_get (data->path, "length", &approx_length, NULL);
if (!(fabs (approx_length - 46341.f) / 46341.f <= 0.15f))
{
if (g_test_verbose ())
g_print ("M 0 0 L 46341 0 - Expected 46341, got %d instead.", approx_length);
return FALSE;
}
set_triangle_path (data); set_triangle_path (data);
g_object_get (data->path, "length", &approx_length, NULL); g_object_get (data->path, "length", &approx_length, NULL);
/* Allow 15% margin of error */ /* Allow 15% margin of error */
return fabs (approx_length - actual_length) / actual_length <= 0.15f; if (!(fabs (approx_length - actual_length) / (float) actual_length <= 0.15f))
{
if (g_test_verbose ())
g_print ("Expected %g, got %d instead.\n", actual_length, approx_length);
return FALSE;
}
return TRUE;
} }
static gboolean static gboolean