fix:core:Fix buffer overflow for ticket #1167 (#1170)

* Refactor:Transform: Create transform func for a single point

we have a common pattern where we call transform() for a single point. We have
to specify the last 4 parameters as constants in a function with too
many parameters. So refactor this function to a simpler signature. While
at this, we also rename the function for easy distinction.

* Fix:Transform: Fix buffer overflow in transform_point_buf in ticket #1167

When displayitem_transform_holes() is called, we allocate a struct point buffer
of size count. Then we call transform_point_buf() to fill that buffer called
result. In this function we fill the buffer in a for loop that runs count times.
The buffer is indexed using result_idx which is incremented every loop
iteration. However, if we are in 3d mode (indicated by t->ddd), we call
transform_z_clip_if_necessary(). This can lead to the repetition of the current
loop iteration by decreasing the loop variable i by 1. Even though we decreased
i we still increment result_idx by 1. So from the point of view of result_idx we
are running the loop count+1 times. Thus, we write one element past the
allocated buffer.

To fix this we give the size of the allocated buffer to
transform_point_buf(). Then we check in the loop if the repetition of this
loop iteration would fit into the buffer. If not, we double the size of
the buffer and try again until we succeed.

Co-authored-by: Stefan Wildemann <metalstrolch@users.noreply.github.com>

5 files changed, 54 insertions, 21 deletions

diff --git a/navit/graphics.c b/navit/graphics.c
index 2c7cd49ff..aabe9b7bc 100644
--- a/navit/graphics.c
+++ b/navit/graphics.c
@@ -2691,15 +2691,26 @@ static inline void displayitem_transform_holes(struct transformation *trans, enu
     out->ccount=NULL;
     out->coords=NULL;
     if((in != NULL) && (in->count > 0)) {
-        int a;
+        int a, transform_res;
         /* alloc space for hole conversion. To be freed with displayitem_free_holes later*/
         out->count = in->count;
         out->ccount = g_malloc0(sizeof(*(out->ccount)) * in->count);
         out->coords = g_malloc0(sizeof(*(out->coords)) * in->count);
         for(a = 0; a < in->count; a ++) {
+            int buf_size=sizeof(*(out->coords[a])) * in->ccount[a];
             in->ccount[a]=limit_count(in->coords[a], in->ccount[a]);
-            out->coords[a]=g_malloc0(sizeof(*(out->coords[a])) * in->ccount[a]);
-            out->ccount[a]=transform(trans, pro, in->coords[a], (struct point *)(out->coords[a]), in->ccount[a], mindist, 0, NULL);
+            out->coords[a]=g_malloc0(buf_size);
+            transform_res=transform_point_buf(trans, pro, in->coords[a], (struct point *)(out->coords[a]), buf_size, in->ccount[a],
+                                              mindist, 0, NULL);
+            /* if we did not have enough buf space for transfrom_point_buf, we try again with double the buffer size,
+               until we succeed. */
+            while (transform_res == TRANSFORM_ERR_BUF_SPACE) {
+                buf_size *= 2;
+                out->coords[a] = g_realloc(out->coords[a], buf_size);
+                transform_res=transform_point_buf(trans, pro, in->coords[a], (struct point *)(out->coords[a]), buf_size,
+                                                  in->ccount[a], mindist, 0, NULL);
+            }
+            out->ccount[a] = transform_res;
         }
     }
 }
@@ -2883,13 +2894,14 @@ static void displayitem_draw(struct displayitem *di, struct layout *l, struct di
     struct graphics *gra=dc->gra;
     struct element *e=dc->e;
     int draw_underground=0;
+    long pa_buf_size=sizeof(struct point)*dc->maxlen;
 
     if (dc->maxlen < ALLOCA_COORD_LIMIT) {
         width=g_alloca(sizeof(int)*dc->maxlen);
-        pa=g_alloca(sizeof(struct point)*dc->maxlen);
+        pa=g_alloca(pa_buf_size);
     } else {
         width=g_malloc(sizeof(int)*dc->maxlen);
-        pa=g_malloc(sizeof(struct point)*dc->maxlen);
+        pa=g_malloc(pa_buf_size);
     }
 
     while (di) {
@@ -2937,11 +2949,13 @@ static void displayitem_draw(struct displayitem *di, struct layout *l, struct di
         if (dc->type == type_poly_water_tiled)
             mindist=0;
         if (dc->e->type == element_polyline)
-            count=transform(dc->trans, dc->pro, di->c, pa, count, mindist, e->u.polyline.width, width);
+            count=transform_point_buf(dc->trans, dc->pro, di->c, pa, pa_buf_size, count, mindist, e->u.polyline.width,
+                                      width);
         else if (dc->e->type == element_arrows)
-            count=transform(dc->trans, dc->pro, di->c, pa, count, mindist, e->u.arrows.width, width);
+            count=transform_point_buf(dc->trans, dc->pro, di->c, pa, pa_buf_size, count, mindist, e->u.arrows.width,
+                                      width);
         else
-            count=transform(dc->trans, dc->pro, di->c, pa, count, mindist, 0, NULL);
+            count=transform_point_buf(dc->trans, dc->pro, di->c, pa, pa_buf_size, count, mindist, 0, NULL);
         switch (e->type) {
         case element_polygon:
             displayitem_draw_polygon(dc, gra, pa, count, &t_holes);
@@ -3699,13 +3713,15 @@ int graphics_displayitem_within_dist(struct displaylist *displaylist, struct dis
     int result;
     struct point *pa;
     int count;
+    long pa_buf_size=sizeof(struct point)*displaylist->dc.maxlen;
+
     if (displaylist->dc.maxlen < ALLOCA_COORD_LIMIT) {
-        pa=g_alloca(sizeof(struct point)*displaylist->dc.maxlen);
+        pa=g_alloca(pa_buf_size);
     } else {
-        pa=g_malloc(sizeof(struct point)*displaylist->dc.maxlen);
+        pa=g_malloc(pa_buf_size);
     }
 
-    count=transform(displaylist->dc.trans, displaylist->dc.pro, di->c, pa, di->count, 0, 0, NULL);
+    count=transform_point_buf(displaylist->dc.trans, displaylist->dc.pro, di->c, pa, pa_buf_size, di->count, 0, 0, NULL);
 
     if (di->item.type < type_line) {
         result =  within_dist_point(p, &pa[0], dist);
diff --git a/navit/gui/internal/gui_internal.c b/navit/gui/internal/gui_internal.c
index bb28c2e84..505432da9 100644
--- a/navit/gui/internal/gui_internal.c
+++ b/navit/gui/internal/gui_internal.c
@@ -1235,7 +1235,7 @@ void gui_internal_cmd_position_do(struct gui_priv *this, struct pcoord *pc_in, s
         c.y=pc.y;
 
         trans=navit_get_trans(this->nav);
-        transform(trans,pc.pro,&c,&p,1,0,0,0);
+        transform_point(trans,pc.pro,&c,&p);
         display=navit_get_displaylist(this->nav);
         dlh=graphics_displaylist_open(display);
         sel=displaylist_get_selection(display);
diff --git a/navit/navit.c b/navit/navit.c
index 14087c4ed..fc1c973a0 100644
--- a/navit/navit.c
+++ b/navit/navit.c
@@ -779,7 +779,7 @@ static void navit_autozoom(struct navit *this_, struct coord *center, int speed,
     distance = speed * this_->autozoom_secs;
 
     transform_get_size(this_->trans, &w, &h);
-    transform(this_->trans, transform_get_projection(this_->trans), center, &pc, 1, 0, 0, NULL);
+    transform_point(this_->trans, transform_get_projection(this_->trans), center, &pc);
     scale = transform_get_scale(this_->trans);
 
     /* We make sure that the point we want to see is within a certain range
@@ -2277,8 +2277,8 @@ void navit_zoom_to_rect(struct navit *this_, struct coord_rect *r) {
         struct point p1,p2;
         transform_set_scale(this_->trans, scale);
         transform_setup_source_rect(this_->trans);
-        transform(this_->trans, transform_get_projection(this_->trans), &r->lu, &p1, 1, 0, 0, NULL);
-        transform(this_->trans, transform_get_projection(this_->trans), &r->rl, &p2, 1, 0, 0, NULL);
+        transform_point(this_->trans, transform_get_projection(this_->trans), &r->lu, &p1);
+        transform_point(this_->trans, transform_get_projection(this_->trans), &r->rl, &p2);
         dbg(lvl_debug,"%d,%d-%d,%d",p1.x,p1.y,p2.x,p2.y);
         if (p1.x < 0 || p2.x < 0 || p1.x > w || p2.x > w ||
                 p1.y < 0 || p2.y < 0 || p1.y > h || p2.y > h)
@@ -3224,7 +3224,7 @@ static void navit_vehicle_draw(struct navit *this_, struct navit_vehicle *nv, st
         pro=transform_get_projection(this_->trans_cursor);
         if (!pro)
             return;
-        transform(this_->trans_cursor, pro, &nv->coord, &cursor_pnt, 1, 0, 0, NULL);
+        transform_point(this_->trans_cursor, pro, &nv->coord, &cursor_pnt);
     }
     vehicle_draw(nv->vehicle, this_->gra, &cursor_pnt, nv->dir-transform_get_yaw(this_->trans_cursor), nv->speed);
 }
@@ -3309,7 +3309,7 @@ static void navit_vehicle_update_position(struct navit *this_, struct navit_vehi
         if (this_->gui && nv->speed > 2)
             navit_disable_suspend();
 
-        transform(this_->trans_cursor, pro, &nv->coord, &cursor_pnt, 1, 0, 0, NULL);
+        transform_point(this_->trans_cursor, pro, &nv->coord, &cursor_pnt);
         if (this_->button_pressed != 1 && this_->follow_cursor && nv->follow_curr <= nv->follow &&
                 (nv->follow_curr == 1 || !transform_within_border(this_->trans_cursor, &cursor_pnt, this_->border)))
             navit_set_center_cursor_draw(this_);
diff --git a/navit/transform.c b/navit/transform.c
index 771037f8c..0221c3070 100644
--- a/navit/transform.c
+++ b/navit/transform.c
@@ -545,14 +545,21 @@ static struct z_clip_result transform_z_clip_if_necessary(struct coord_3d coord,
     return clip_result;
 }
 
-int transform(struct transformation *t, enum projection required_projection, struct coord *input,
-              struct point *result, int count, int mindist, int width, int *width_result) {
+int transform_point(struct transformation *t, enum projection required_projection, struct coord *input,
+                    struct point *result) {
+    return transform_point_buf(t, required_projection, input, result, sizeof(struct point), 1, 0, 0, NULL);
+}
+
+int transform_point_buf(struct transformation *t, enum projection required_projection, struct coord *input,
+                        struct point *result, long result_size, int count, int mindist, int width, int *width_result) {
     struct coord projected_coord, shifted_coord;
     struct coord_3d rotated_coord;
     struct point screen_point;
     int zlimit=t->znear;
     struct z_clip_result clip_result, clip_result_old= {{0,0}, -1, 0, 0};
     int i,result_idx = 0,result_idx_last=0;
+    long max_results = result_size / sizeof(struct point);
+
     dbg(lvl_debug,"count=%d", count);
     for (i=0; i < count; i++) {
         dbg(lvl_debug, "input coord %d: (%d, %d)", i, input[i].x, input[i].y);
@@ -570,7 +577,14 @@ int transform(struct transformation *t, enum projection required_projection, str
             clip_result=transform_z_clip_if_necessary(rotated_coord, zlimit, clip_result_old);
             clip_result_old=clip_result;
             if(clip_result.process_coord_again) {
-                i--;
+                /* if we repeat an interation, we have to make sure that there is enough space in the result buffer to
+                   not overflow. */
+                if (result_idx + 1  < max_results) {
+                    i--;
+                } else {
+                    dbg(lvl_debug, "Not enough space in buf for transform_point_buf");
+                    return TRANSFORM_ERR_BUF_SPACE;
+                }
             } else if (clip_result.skip_coord) {
                 continue;
             }
diff --git a/navit/transform.h b/navit/transform.h
index dab69fd47..e492882a0 100644
--- a/navit/transform.h
+++ b/navit/transform.h
@@ -25,6 +25,8 @@ extern "C" {
 #endif
 #include "coord.h"
 
+#define TRANSFORM_ERR_BUF_SPACE -1
+
 /* prototypes */
 enum attr_type;
 enum item_type;
@@ -55,7 +57,8 @@ void transform_geo_to_cart(struct coord_geo *geo, navit_float a, navit_float b,
 void transform_cart_to_geo(struct coord_geo_cart *cart, navit_float a, navit_float b, struct coord_geo *geo);
 void transform_utm_to_geo(const double UTMEasting, const double UTMNorthing, int ZoneNumber, int NorthernHemisphere, struct coord_geo *geo);
 void transform_datum(struct coord_geo *from, enum map_datum from_datum, struct coord_geo *to, enum map_datum to_datum);
-int transform(struct transformation *t, enum projection pro, struct coord *c, struct point *p, int count, int mindist, int width, int *width_return);
+int transform_point(struct transformation *t, enum projection pro, struct coord *c, struct point *p);
+int transform_point_buf(struct transformation *t, enum projection pro, struct coord *c, struct point *p, long result_size, int count, int mindist, int width, int *width_return);
 int transform_reverse(struct transformation *t, struct point *p, struct coord *c);
 double transform_pixels_to_map_distance(struct transformation *transformation, int pixels);
 enum projection transform_get_projection(struct transformation *this_);