From: Emmanuele Bassi <ebassi@linux.intel.com>
Date: Mon, 23 Apr 2012 16:42:40 +0000 (+0100)
Subject: path: Avoid integer overflow in get_distance()
X-Git-Url: http://review.tizen.org/git/?a=commitdiff_plain;h=0fca11ec2f38e15fda2a27f31f39110c91f2a4f0;p=profile%2Fivi%2Fclutter.git

path: Avoid integer overflow in get_distance()

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
---

diff --git a/clutter/clutter-path.c b/clutter/clutter-path.c
index 0364b1b..799c2e5 100644
--- a/clutter/clutter-path.c
+++ b/clutter/clutter-path.c
@@ -632,14 +632,14 @@ clutter_path_parse_description (const gchar  *p,
           node = clutter_path_node_full_new ();
           nodes = g_slist_prepend (nodes, node);
 
-          node->k.type = (*p == 'M' ? CLUTTER_PATH_MOVE_TO
-                          : *p == 'm' ? CLUTTER_PATH_REL_MOVE_TO
-                          : *p == 'L' ? CLUTTER_PATH_LINE_TO
-                          : CLUTTER_PATH_REL_LINE_TO);
+          node->k.type = (*p == 'M' ? CLUTTER_PATH_MOVE_TO :
+                          *p == 'm' ? CLUTTER_PATH_REL_MOVE_TO :
+                          *p == 'L' ? CLUTTER_PATH_LINE_TO :
+                          CLUTTER_PATH_REL_LINE_TO);
           p++;
 
-          if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x)
-              || !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y))
+          if (!clutter_path_parse_number (&p, FALSE, &node->k.points[0].x) ||
+              !clutter_path_parse_number (&p, TRUE, &node->k.points[0].y))
             goto fail;
           break;
 
@@ -648,16 +648,16 @@ clutter_path_parse_description (const gchar  *p,
           node = clutter_path_node_full_new ();
           nodes = g_slist_prepend (nodes, node);
 
-          node->k.type = (*p == 'C' ? CLUTTER_PATH_CURVE_TO
-                          : CLUTTER_PATH_REL_CURVE_TO);
+          node->k.type = (*p == 'C' ? CLUTTER_PATH_CURVE_TO :
+                          CLUTTER_PATH_REL_CURVE_TO);
           p++;
 
-          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[1].x)
-              || !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].y))
+          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[1].x) ||
+              !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].y))
             goto fail;
           break;
 
@@ -1244,11 +1244,12 @@ clutter_path_get_description (ClutterPath *path)
   return g_string_free (str, FALSE);
 }
 
-static gint
+static guint
 clutter_path_node_distance (const ClutterKnot *start,
                             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 (end != NULL, 0);
@@ -1256,10 +1257,12 @@ clutter_path_node_distance (const ClutterKnot *start,
   if (clutter_knot_equal (start, end))
     return 0;
 
-  t = (end->x - start->x) * (end->x - start->x) +
-    (end->y - start->y) * (end->y - start->y);
+  x_d = end->x - start->x;
+  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
diff --git a/tests/conform/path.c b/tests/conform/path.c
index 25c81fb..b292ac9 100644
--- a/tests/conform/path.c
+++ b/tests/conform/path.c
@@ -551,12 +551,42 @@ path_test_get_length (CallbackData *data)
   const float actual_length /* sqrt(64**2 + 64**2) * 2 */ = 181.019336f;
   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);
 
   g_object_get (data->path, "length", &approx_length, NULL);
 
   /* 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