From 7878dd6d4bbf8ff9463dcf827c2f05be3525d77a Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Thu, 2 Apr 2020 20:59:19 +0300 Subject: [PATCH 1/3] Fix several out-of-bound reads; fix a memory leak --- tools/gfx.c | 20 ++++++++++++++++---- 1 file changed, 16 insertions(+), 4 deletions(-) diff --git a/tools/gfx.c b/tools/gfx.c index 8c4066ab3..b1f9aebad 100644 --- a/tools/gfx.c +++ b/tools/gfx.c @@ -102,9 +102,13 @@ void trim_whitespace(struct Graphic *graphic) { void remove_whitespace(struct Graphic *graphic) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + int i = 0; for (int j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { - while (is_whitespace(&graphic->data[j], tile_size)) { + while (j < graphic->size && is_whitespace(&graphic->data[j], tile_size)) { j += tile_size; } if (j >= graphic->size) { @@ -136,8 +140,12 @@ void remove_duplicates(struct Graphic *graphic) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; int num_tiles = 0; + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { - while (tile_exists(&graphic->data[j], graphic->data, tile_size, num_tiles)) { + while (j < graphic->size && tile_exists(&graphic->data[j], graphic->data, tile_size, num_tiles)) { if (Options.keep_whitespace && is_whitespace(&graphic->data[j], tile_size)) { break; } @@ -155,7 +163,7 @@ void remove_duplicates(struct Graphic *graphic) { } bool flip_exists(uint8_t *tile, uint8_t *tiles, int tile_size, int num_tiles, bool xflip, bool yflip) { - uint8_t *flip = calloc(tile_size, 1); + uint8_t *flip = alloca(tile_size); int half_size = tile_size / 2; for (int i = 0; i < tile_size; i++) { int byte = i; @@ -183,8 +191,12 @@ void remove_flip(struct Graphic *graphic, bool xflip, bool yflip) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; int num_tiles = 0; + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { - while (flip_exists(&graphic->data[j], graphic->data, tile_size, num_tiles, xflip, yflip)) { + while (j < graphic->size && flip_exists(&graphic->data[j], graphic->data, tile_size, num_tiles, xflip, yflip)) { if (Options.keep_whitespace && is_whitespace(&graphic->data[j], tile_size)) { break; } From 79f3f3f4d5bb0fbe0d75596960ab72dcaffc16de Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Thu, 2 Apr 2020 21:10:20 +0300 Subject: [PATCH 2/3] Indentation conventions, use C99 variable length arrays --- tools/gfx.c | 26 +++++++++++++------------- 1 file changed, 13 insertions(+), 13 deletions(-) diff --git a/tools/gfx.c b/tools/gfx.c index b1f9aebad..a3012c088 100644 --- a/tools/gfx.c +++ b/tools/gfx.c @@ -102,10 +102,10 @@ void trim_whitespace(struct Graphic *graphic) { void remove_whitespace(struct Graphic *graphic) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; - - // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; - + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + int i = 0; for (int j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { while (j < graphic->size && is_whitespace(&graphic->data[j], tile_size)) { @@ -140,10 +140,10 @@ void remove_duplicates(struct Graphic *graphic) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; int num_tiles = 0; - - // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; - + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { while (j < graphic->size && tile_exists(&graphic->data[j], graphic->data, tile_size, num_tiles)) { if (Options.keep_whitespace && is_whitespace(&graphic->data[j], tile_size)) { @@ -163,7 +163,7 @@ void remove_duplicates(struct Graphic *graphic) { } bool flip_exists(uint8_t *tile, uint8_t *tiles, int tile_size, int num_tiles, bool xflip, bool yflip) { - uint8_t *flip = alloca(tile_size); + uint8_t flip[tile_size]; int half_size = tile_size / 2; for (int i = 0; i < tile_size; i++) { int byte = i; @@ -191,10 +191,10 @@ void remove_flip(struct Graphic *graphic, bool xflip, bool yflip) { int tile_size = Options.depth * 8; if (Options.interleave) tile_size *= 2; int num_tiles = 0; - - // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; - + + // Make sure we have a whole number of tiles, round down if required + graphic->size &= tile_size - 1; + for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { while (j < graphic->size && flip_exists(&graphic->data[j], graphic->data, tile_size, num_tiles, xflip, yflip)) { if (Options.keep_whitespace && is_whitespace(&graphic->data[j], tile_size)) { From 2af6c6325365043164ac6afc4cee3d808c2ce1c7 Mon Sep 17 00:00:00 2001 From: Lior Halphon Date: Thu, 2 Apr 2020 21:42:12 +0300 Subject: [PATCH 3/3] Fix bad round, memset VLA --- tools/gfx.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/tools/gfx.c b/tools/gfx.c index a3012c088..7ef0d2d91 100644 --- a/tools/gfx.c +++ b/tools/gfx.c @@ -104,7 +104,7 @@ void remove_whitespace(struct Graphic *graphic) { if (Options.interleave) tile_size *= 2; // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; + graphic->size &= ~(tile_size - 1); int i = 0; for (int j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { @@ -142,7 +142,7 @@ void remove_duplicates(struct Graphic *graphic) { int num_tiles = 0; // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; + graphic->size &= ~(tile_size - 1); for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { while (j < graphic->size && tile_exists(&graphic->data[j], graphic->data, tile_size, num_tiles)) { @@ -164,6 +164,7 @@ void remove_duplicates(struct Graphic *graphic) { bool flip_exists(uint8_t *tile, uint8_t *tiles, int tile_size, int num_tiles, bool xflip, bool yflip) { uint8_t flip[tile_size]; + memset(flip, 0, sizeof(flip)); int half_size = tile_size / 2; for (int i = 0; i < tile_size; i++) { int byte = i; @@ -193,7 +194,7 @@ void remove_flip(struct Graphic *graphic, bool xflip, bool yflip) { int num_tiles = 0; // Make sure we have a whole number of tiles, round down if required - graphic->size &= tile_size - 1; + graphic->size &= ~(tile_size - 1); for (int i = 0, j = 0; i < graphic->size && j < graphic->size; i += tile_size, j += tile_size) { while (j < graphic->size && flip_exists(&graphic->data[j], graphic->data, tile_size, num_tiles, xflip, yflip)) {