From 91783e63587918794c71431c9437c52b2d336d89 Mon Sep 17 00:00:00 2001 From: Peter Bay Date: Mon, 14 Sep 2026 16:25:58 +0200 Subject: [PATCH] displayio, tilepalettemapper, vectorio: arguments narrowed before checked TileGrid put tile_width, tile_height, width and height into uint16_t fields before anything looked at them, so 65536 truncated to 0 -- which for the tile size is the "use the whole bitmap" value and for the grid size gives no tiles at all. default_tile was never bounded against the bitmap it indexes, and the tile count was multiplied in int, which overflows for a large grid and was then used as the size of an allocation. A Group's scale is a uint16_t and its constructor accepts up to 32767, but absolute_transform held it in a uint8_t and the step in an int8_t. Those are widened, the products computed in uint32_t and clamped, and the setter given the upper bound the constructor always implied. The palette bounds check admitted an index equal to the colour count, which is already one past the end. TilePaletteMapper accepted zero colours per tile, and width_in_tiles is only assigned when it is bound to a TileGrid, so a subscript before that divided by zero. An input pixel past input_color_count also indexed the mapping row unchecked. vectorio.Polygon takes its point count as a size_t but keeps twice it in a uint16_t, and counted the validation loop with a uint16_t as well. --- shared-bindings/displayio/Group.c | 2 +- shared-bindings/displayio/TileGrid.c | 17 ++++++++++++++++- .../tilepalettemapper/TilePaletteMapper.c | 7 ++++++- shared-module/displayio/Group.c | 8 +++++--- shared-module/displayio/Palette.c | 2 +- shared-module/displayio/TileGrid.c | 4 +++- shared-module/displayio/area.h | 6 +++--- .../tilepalettemapper/TilePaletteMapper.c | 4 ++++ shared-module/vectorio/Polygon.c | 3 ++- 9 files changed, 41 insertions(+), 12 deletions(-) diff --git a/shared-bindings/displayio/Group.c b/shared-bindings/displayio/Group.c index 94ee2e58de1..702b1fca8e0 100644 --- a/shared-bindings/displayio/Group.c +++ b/shared-bindings/displayio/Group.c @@ -87,7 +87,7 @@ MP_DEFINE_CONST_FUN_OBJ_1(displayio_group_get_scale_obj, displayio_group_obj_get static mp_obj_t displayio_group_obj_set_scale(mp_obj_t self_in, mp_obj_t scale_obj) { displayio_group_t *self = native_group(self_in); - mp_int_t scale = mp_arg_validate_int_min(mp_obj_get_int(scale_obj), 1, MP_QSTR_scale); + mp_int_t scale = mp_arg_validate_int_range(mp_obj_get_int(scale_obj), 1, 32767, MP_QSTR_scale); common_hal_displayio_group_set_scale(self, scale); return mp_const_none; diff --git a/shared-bindings/displayio/TileGrid.c b/shared-bindings/displayio/TileGrid.c index fb5cabf05bf..bc751b472a9 100644 --- a/shared-bindings/displayio/TileGrid.c +++ b/shared-bindings/displayio/TileGrid.c @@ -110,6 +110,8 @@ static mp_obj_t displayio_tilegrid_make_new(const mp_obj_type_t *type, size_t n_ } mp_obj_t pixel_shader = args[ARG_pixel_shader].u_obj; displayio_tilegrid_validate_pixel_shader(pixel_shader); + mp_arg_validate_int_range(args[ARG_tile_width].u_int, 0, 0xffff, MP_QSTR_tile_width); + mp_arg_validate_int_range(args[ARG_tile_height].u_int, 0, 0xffff, MP_QSTR_tile_height); uint16_t tile_width = args[ARG_tile_width].u_int; if (tile_width == 0) { tile_width = bitmap_width; @@ -118,6 +120,8 @@ static mp_obj_t displayio_tilegrid_make_new(const mp_obj_type_t *type, size_t n_ if (tile_height == 0) { tile_height = bitmap_height; } + mp_arg_validate_int_min(tile_width, 1, MP_QSTR_tile_width); + mp_arg_validate_int_min(tile_height, 1, MP_QSTR_tile_height); if (bitmap_width % tile_width != 0) { mp_raise_ValueError(MP_ERROR_TEXT("Tile width must exactly divide bitmap width")); } @@ -125,12 +129,23 @@ static mp_obj_t displayio_tilegrid_make_new(const mp_obj_type_t *type, size_t n_ mp_raise_ValueError(MP_ERROR_TEXT("Tile height must exactly divide bitmap height")); } + mp_arg_validate_int_range(args[ARG_width].u_int, 1, 0xffff, MP_QSTR_width); + mp_arg_validate_int_range(args[ARG_height].u_int, 1, 0xffff, MP_QSTR_height); + + uint16_t bitmap_width_in_tiles = bitmap_width / tile_width; + uint16_t bitmap_height_in_tiles = bitmap_height / tile_height; + uint32_t tiles_in_bitmap = (uint32_t)bitmap_width_in_tiles * bitmap_height_in_tiles; + mp_arg_validate_length_min(tiles_in_bitmap, 1, MP_QSTR_bitmap); + + mp_arg_validate_int_range(args[ARG_default_tile].u_int, 0, + (mp_int_t)tiles_in_bitmap - 1, MP_QSTR_default_tile); + int16_t x = args[ARG_x].u_int; int16_t y = args[ARG_y].u_int; displayio_tilegrid_t *self = mp_obj_malloc(displayio_tilegrid_t, &displayio_tilegrid_type); common_hal_displayio_tilegrid_construct(self, bitmap, - bitmap_width / tile_width, bitmap_height / tile_height, + bitmap_width_in_tiles, bitmap_height_in_tiles, pixel_shader, args[ARG_width].u_int, args[ARG_height].u_int, tile_width, tile_height, x, y, args[ARG_default_tile].u_int); diff --git a/shared-bindings/tilepalettemapper/TilePaletteMapper.c b/shared-bindings/tilepalettemapper/TilePaletteMapper.c index c22ea47bf26..a4438bb1eca 100644 --- a/shared-bindings/tilepalettemapper/TilePaletteMapper.c +++ b/shared-bindings/tilepalettemapper/TilePaletteMapper.c @@ -45,6 +45,8 @@ static mp_obj_t tilepalettemapper_tilepalettemapper_make_new(const mp_obj_type_t mp_raise_TypeError_varg(MP_ERROR_TEXT("unsupported %q type"), MP_QSTR_pixel_shader); } + mp_arg_validate_int_min(args[ARG_input_color_count].u_int, 1, MP_QSTR_input_color_count); + tilepalettemapper_tilepalettemapper_t *self = mp_obj_malloc(tilepalettemapper_tilepalettemapper_t, &tilepalettemapper_tilepalettemapper_type); common_hal_tilepalettemapper_tilepalettemapper_construct(self, pixel_shader, args[ARG_input_color_count].u_int); @@ -128,9 +130,12 @@ static mp_obj_t tilepalettemapper_subscr(mp_obj_t self_in, mp_obj_t index_obj, m } else { uint16_t x = 0; uint16_t y = 0; + uint16_t width = common_hal_tilepalettemapper_tilepalettemapper_get_width(self); + if (width == 0) { + mp_raise_IndexError(MP_ERROR_TEXT("Tile index out of bounds")); + } if (mp_obj_is_small_int(index_obj)) { mp_int_t i = MP_OBJ_SMALL_INT_VALUE(index_obj); - uint16_t width = common_hal_tilepalettemapper_tilepalettemapper_get_width(self); x = i % width; y = i / width; } else { diff --git a/shared-module/displayio/Group.c b/shared-module/displayio/Group.c index 84ebffdab6d..1f6d8034b5e 100644 --- a/shared-module/displayio/Group.c +++ b/shared-module/displayio/Group.c @@ -187,7 +187,8 @@ void displayio_group_update_transform(displayio_group_t *self, self->absolute_transform.mirror_x = parent_transform->mirror_x; self->absolute_transform.mirror_y = parent_transform->mirror_y; - self->absolute_transform.scale = parent_transform->scale * self->scale; + uint32_t combined = (uint32_t)parent_transform->scale * self->scale; + self->absolute_transform.scale = combined > UINT16_MAX ? UINT16_MAX : (uint16_t)combined; } _update_child_transforms(self); } @@ -197,10 +198,11 @@ void common_hal_displayio_group_set_scale(displayio_group_t *self, uint32_t scal return; } check_readonly(self); - uint8_t parent_scale = self->absolute_transform.scale / self->scale; + uint16_t parent_scale = self->absolute_transform.scale / self->scale; self->absolute_transform.dx = self->absolute_transform.dx / self->scale * scale; self->absolute_transform.dy = self->absolute_transform.dy / self->scale * scale; - self->absolute_transform.scale = parent_scale * scale; + uint32_t combined = (uint32_t)parent_scale * scale; + self->absolute_transform.scale = combined > UINT16_MAX ? UINT16_MAX : (uint16_t)combined; self->scale = scale; _update_child_transforms(self); } diff --git a/shared-module/displayio/Palette.c b/shared-module/displayio/Palette.c index 968296c69f1..bb0035f1eb5 100644 --- a/shared-module/displayio/Palette.c +++ b/shared-module/displayio/Palette.c @@ -55,7 +55,7 @@ uint32_t common_hal_displayio_palette_get_color(displayio_palette_t *self, uint3 void displayio_palette_get_color(displayio_palette_t *self, const _displayio_colorspace_t *colorspace, const displayio_input_pixel_t *input_pixel, displayio_output_pixel_t *output_color) { uint32_t palette_index = input_pixel->pixel; - if (palette_index > self->color_count || self->colors[palette_index].transparent) { + if (palette_index >= self->color_count || self->colors[palette_index].transparent) { output_color->opaque = false; return; } diff --git a/shared-module/displayio/TileGrid.c b/shared-module/displayio/TileGrid.c index b5c691ccbd6..f0365e9b08f 100644 --- a/shared-module/displayio/TileGrid.c +++ b/shared-module/displayio/TileGrid.c @@ -22,7 +22,9 @@ void common_hal_displayio_tilegrid_construct(displayio_tilegrid_t *self, mp_obj_ mp_obj_t pixel_shader, uint16_t width, uint16_t height, uint16_t tile_width, uint16_t tile_height, uint16_t x, uint16_t y, uint16_t default_tile) { - uint32_t total_tiles = width * height; + uint32_t total_tiles = (uint32_t)width * (uint32_t)height; + mp_arg_validate_length_max(total_tiles, SIZE_MAX / sizeof(uint16_t), + MP_QSTR_TileGrid); self->bitmap_width_in_tiles = bitmap_width_in_tiles; self->tiles_in_bitmap = bitmap_width_in_tiles * bitmap_height_in_tiles; diff --git a/shared-module/displayio/area.h b/shared-module/displayio/area.h index eed54613a5a..88c952e58e3 100644 --- a/shared-module/displayio/area.h +++ b/shared-module/displayio/area.h @@ -23,9 +23,9 @@ struct _displayio_area_t { typedef struct { uint16_t x; uint16_t y; - int8_t dx; - int8_t dy; - uint8_t scale; + int16_t dx; + int16_t dy; + uint16_t scale; uint16_t width; uint16_t height; bool mirror_x; diff --git a/shared-module/tilepalettemapper/TilePaletteMapper.c b/shared-module/tilepalettemapper/TilePaletteMapper.c index 13711b2c253..bb0a0acc199 100644 --- a/shared-module/tilepalettemapper/TilePaletteMapper.c +++ b/shared-module/tilepalettemapper/TilePaletteMapper.c @@ -67,6 +67,10 @@ void tilepalettemapper_tilepalettemapper_get_color(tilepalettemapper_tilepalette } return; } + if (input_pixel->pixel >= self->input_color_count) { + output_color->opaque = false; + return; + } uint16_t tile_index = y_tile_index * self->width_in_tiles + x_tile_index; uint32_t mapped_index = self->tile_mappings[tile_index][input_pixel->pixel]; displayio_input_pixel_t tmp_pixel; diff --git a/shared-module/vectorio/Polygon.c b/shared-module/vectorio/Polygon.c index 212d519e298..0c5c487b156 100644 --- a/shared-module/vectorio/Polygon.c +++ b/shared-module/vectorio/Polygon.c @@ -31,6 +31,7 @@ static void _clobber_points_list(vectorio_polygon_t *self, mp_obj_t points_tuple if (len < 3) { mp_raise_TypeError(MP_ERROR_TEXT("Polygon needs at least 3 points")); } + mp_arg_validate_length_range(len, 3, UINT16_MAX / 2, MP_QSTR_points); int16_t *points_list = gc_realloc(self->points_list, 2 * len * sizeof(uint16_t), true); VECTORIO_POLYGON_DEBUG("realloc(%p, %d) -> %p", self->points_list, 2 * len * sizeof(uint16_t), points_list); @@ -39,7 +40,7 @@ static void _clobber_points_list(vectorio_polygon_t *self, mp_obj_t points_tuple self->points_list = NULL; self->len = 0; - for (uint16_t i = 0; i < len; ++i) { + for (size_t i = 0; i < len; ++i) { size_t tuple_len = 0; mp_obj_t *tuple_items; mp_arg_validate_type(items[i], &mp_type_tuple, MP_QSTR_point);