Skip to content

Commit

Permalink
fix:core:Fix buffer overflow for ticket navit-gps#1167 (navit-gps#1170)
Browse files Browse the repository at this point in the history 
* 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 navit-gps#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 <[email protected]>
  • Loading branch information
@bkoppelmann @metalstrolch
bkoppelmann and metalstrolch authored Dec 23, 2021
1 parent a671d30 commit 13e550f
Show file tree
Hide file tree
Showing 5 changed files with 54 additions and 21 deletions.
38 changes: 27 additions & 11 deletions navit/graphics.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
}
}
}
Expand Down Expand Up @@ -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) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
Expand Down
2 changes: 1 addition & 1 deletion navit/gui/internal/gui_internal.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
10 changes: 5 additions & 5 deletions navit/navit.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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_);
Expand Down
20 changes: 17 additions & 3 deletions navit/transform.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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;
}
Expand Down
5 changes: 4 additions & 1 deletion navit/transform.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,8 @@ extern "C" {
#endif
#include "coord.h"

#define TRANSFORM_ERR_BUF_SPACE -1

/* prototypes */
enum attr_type;
enum item_type;
Expand Down Expand Up @@ -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_);
Expand Down

0 comments on commit 13e550f

Please sign in to comment.