From e28c08d63490a2fb6b8cc07bf968eb16243e9c63 Mon Sep 17 00:00:00 2001 From: Frediano Ziglio Date: Tue, 8 Sep 2015 10:13:24 +0100 Subject: [PATCH 49/57] Fix integer overflow computing glyph_size in red_get_string If bpp is int the formula can lead to weird overflows. width and height are uint16_t so the formula is: size_t = u16 * (u16 * int + const_int) / const_int; so it became size_t = (int) u16 * ((int) u16 * int + const_int) / const_int; However the (int) u16 * (int) u16 can then became negative to overflow. Under 64 bit architectures size_t is 64 and int usually 32 so converting this negative 32 bit number to a unsigned 64 bit lead to a very big number as the signed is extended and then converted to unsigned. Using unsigned arithmetic prevent extending the sign. Signed-off-by: Frediano Ziglio Acked-by: Christophe Fergeau --- server/red_parse_qxl.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c index 2d4636e..c4b82be 100644 --- a/server/red_parse_qxl.c +++ b/server/red_parse_qxl.c @@ -807,7 +807,9 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id, uint8_t *data; bool free_data; size_t chunk_size, qxl_size, red_size, glyph_size; - int glyphs, bpp = 0, i; + int glyphs, i; + /* use unsigned to prevent integer overflow in multiplication below */ + unsigned int bpp = 0; int error; uint16_t qxl_flags, qxl_length; @@ -846,7 +848,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id, while (start < end) { spice_assert((QXLRasterGlyph*)(&start->data[0]) <= end); glyphs++; - glyph_size = start->height * ((start->width * bpp + 7) / 8); + glyph_size = start->height * ((start->width * bpp + 7u) / 8u); red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4); start = (QXLRasterGlyph*)(&start->data[glyph_size]); } @@ -867,7 +869,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id, glyph->height = start->height; red_get_point_ptr(&glyph->render_pos, &start->render_pos); red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin); - glyph_size = glyph->height * ((glyph->width * bpp + 7) / 8); + glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u); spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end); memcpy(glyph->data, start->data, glyph_size); start = (QXLRasterGlyph*)(&start->data[glyph_size]); -- 2.4.3