Skip to content

Commit

Permalink
extmod/modframebuf: Validate FrameBuffer bounds against input buffer.
Browse files Browse the repository at this point in the history 
This ensures that the buffer is large enough for the specified width,
height, bits-per-pixel, and stride.

Also makes the legacy FrameBuffer1 constructor re-use the FrameBuffer
make_new to save some code size.

Fixes issue micropython#12562.

This work was funded through GitHub Sponsors.

Signed-off-by: Jim Mussared <[email protected]>
  • Loading branch information
@jimmo @dpgeorge
jimmo authored and dpgeorge committed Oct 16, 2023
1 parent a1be5e1 commit d040478
Show file tree
Hide file tree
Showing 3 changed files with 143 additions and 35 deletions.
74 changes: 39 additions & 35 deletions extmod/modframebuf.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,42 +273,59 @@ STATIC void fill_rect(const mp_obj_framebuf_t *fb, int x, int y, int w, int h, u
STATIC mp_obj_t framebuf_make_new(const mp_obj_type_t *type, size_t n_args, size_t n_kw, const mp_obj_t *args_in) {
mp_arg_check_num(n_args, n_kw, 4, 5, false);

mp_obj_framebuf_t *o = mp_obj_malloc(mp_obj_framebuf_t, type);
o->buf_obj = args_in[0];

mp_buffer_info_t bufinfo;
mp_get_buffer_raise(args_in[0], &bufinfo, MP_BUFFER_WRITE);
o->buf = bufinfo.buf;
mp_int_t width = mp_obj_get_int(args_in[1]);
mp_int_t height = mp_obj_get_int(args_in[2]);
mp_int_t format = mp_obj_get_int(args_in[3]);
mp_int_t stride = n_args >= 5 ? mp_obj_get_int(args_in[4]) : width;

o->width = mp_obj_get_int(args_in[1]);
o->height = mp_obj_get_int(args_in[2]);
o->format = mp_obj_get_int(args_in[3]);
if (n_args >= 5) {
o->stride = mp_obj_get_int(args_in[4]);
} else {
o->stride = o->width;
if (width < 1 || height < 1 || width > 0xffff || height > 0xffff || stride > 0xffff || stride < width) {
mp_raise_ValueError(NULL);
}

switch (o->format) {
size_t height_required = height;
size_t bpp = 1;

switch (format) {
case FRAMEBUF_MVLSB:
case FRAMEBUF_RGB565:
height_required = (height + 7) & ~7;
break;
case FRAMEBUF_MHLSB:
case FRAMEBUF_MHMSB:
o->stride = (o->stride + 7) & ~7;
stride = (stride + 7) & ~7;
break;
case FRAMEBUF_GS2_HMSB:
o->stride = (o->stride + 3) & ~3;
stride = (stride + 3) & ~3;
bpp = 2;
break;
case FRAMEBUF_GS4_HMSB:
o->stride = (o->stride + 1) & ~1;
stride = (stride + 1) & ~1;
bpp = 4;
break;
case FRAMEBUF_GS8:
bpp = 8;
break;
case FRAMEBUF_RGB565:
bpp = 16;
break;
default:
mp_raise_ValueError(MP_ERROR_TEXT("invalid format"));
}

mp_buffer_info_t bufinfo;
mp_get_buffer_raise(args_in[0], &bufinfo, MP_BUFFER_WRITE);

if (height_required * stride * bpp / 8 > bufinfo.len) {
mp_raise_ValueError(NULL);
}

mp_obj_framebuf_t *o = mp_obj_malloc(mp_obj_framebuf_t, type);
o->buf_obj = args_in[0];
o->buf = bufinfo.buf;
o->width = width;
o->height = height;
o->format = format;
o->stride = stride;

return MP_OBJ_FROM_PTR(o);
}

Expand Down Expand Up @@ -851,24 +868,11 @@ STATIC MP_DEFINE_CONST_OBJ_TYPE(
);
#endif

// this factory function is provided for backwards compatibility with old FrameBuffer1 class
// This factory function is provided for backwards compatibility with the old
// FrameBuffer1 class which did not support a format argument.
STATIC mp_obj_t legacy_framebuffer1(size_t n_args, const mp_obj_t *args_in) {
mp_obj_framebuf_t *o = mp_obj_malloc(mp_obj_framebuf_t, (mp_obj_type_t *)&mp_type_framebuf);

mp_buffer_info_t bufinfo;
mp_get_buffer_raise(args_in[0], &bufinfo, MP_BUFFER_WRITE);
o->buf = bufinfo.buf;

o->width = mp_obj_get_int(args_in[1]);
o->height = mp_obj_get_int(args_in[2]);
o->format = FRAMEBUF_MVLSB;
if (n_args >= 4) {
o->stride = mp_obj_get_int(args_in[3]);
} else {
o->stride = o->width;
}

return MP_OBJ_FROM_PTR(o);
mp_obj_t args[] = {args_in[0], args_in[1], args_in[2], MP_OBJ_NEW_SMALL_INT(FRAMEBUF_MVLSB), n_args >= 4 ? args_in[3] : args_in[1] };
return framebuf_make_new(&mp_type_framebuf, 5, 0, args);
}
STATIC MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(legacy_framebuffer1_obj, 3, 4, legacy_framebuffer1);

Expand Down
60 changes: 60 additions & 0 deletions tests/extmod/framebuf_bounds.py
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
try:
import framebuf
except ImportError:
print("SKIP")
raise SystemExit


def test_constructor(*args):
try:
framebuf.FrameBuffer(*args)
print("Valid")
except ValueError:
print("ValueError")


print(framebuf.MONO_HLSB)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB)
test_constructor(bytearray(20), -1, 10, framebuf.MONO_HLSB)
test_constructor(bytearray(20), 10, -1, framebuf.MONO_HLSB)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 11)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 10)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, 9)
test_constructor(bytearray(20), 10, 10, framebuf.MONO_HLSB, -1)

print(framebuf.MONO_VLSB)
test_constructor(bytearray(8), 8, 1, framebuf.MONO_VLSB)
test_constructor(bytearray(7), 8, 1, framebuf.MONO_VLSB)
test_constructor(bytearray(8), 8, 8, framebuf.MONO_VLSB)

for f in (framebuf.MONO_HLSB, framebuf.MONO_HMSB):
print(f)
test_constructor(bytearray(1), 8, 1, f)
test_constructor(bytearray(0), 8, 1, f)
test_constructor(bytearray(8), 8, 8, f)
test_constructor(bytearray(9), 8, 9, f)
test_constructor(bytearray(9), 9, 8, f)

print(framebuf.GS2_HMSB)
test_constructor(bytearray(8), 4, 8, framebuf.GS2_HMSB)
test_constructor(bytearray(15), 5, 8, framebuf.GS2_HMSB)
test_constructor(bytearray(16), 5, 8, framebuf.GS2_HMSB)
test_constructor(bytearray(9), 4, 9, framebuf.GS2_HMSB)

print(framebuf.GS4_HMSB)
test_constructor(bytearray(8), 2, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(15), 3, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(16), 3, 8, framebuf.GS4_HMSB)
test_constructor(bytearray(9), 2, 9, framebuf.GS4_HMSB)

print(framebuf.GS8)
test_constructor(bytearray(63), 8, 8, framebuf.GS8)
test_constructor(bytearray(64), 8, 8, framebuf.GS8)
test_constructor(bytearray(64), 9, 8, framebuf.GS8)
test_constructor(bytearray(64), 8, 9, framebuf.GS8)

print(framebuf.RGB565)
test_constructor(bytearray(127), 8, 8, framebuf.RGB565)
test_constructor(bytearray(128), 8, 8, framebuf.RGB565)
test_constructor(bytearray(128), 9, 8, framebuf.RGB565)
test_constructor(bytearray(128), 8, 9, framebuf.RGB565)
44 changes: 44 additions & 0 deletions tests/extmod/framebuf_bounds.py.exp
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
3
Valid
ValueError
ValueError
Valid
Valid
ValueError
ValueError
0
Valid
ValueError
Valid
3
Valid
ValueError
Valid
Valid
ValueError
4
Valid
ValueError
Valid
Valid
ValueError
5
Valid
ValueError
Valid
Valid
2
Valid
ValueError
Valid
Valid
6
ValueError
Valid
ValueError
ValueError
1
ValueError
Valid
ValueError
ValueError

0 comments on commit d040478

Please sign in to comment.