From 444d7bacbec51321cea955802b62dff9318dcbf6 Mon Sep 17 00:00:00 2001 From: stijn Date: Wed, 8 Nov 2023 13:43:28 +0100 Subject: [PATCH] extmod/moductypes: Validate the descriptor tuple. Fixes various null dereferencing, out-of-bounds memory accesses and `assert(0)` failures in the case of an invalid `uctypes` descriptor. By design `uctypes` can crash because it accesses arbitrary memory, but at least describing the descriptor layout should be forced to be correct and not crash. Fixes issue #12702. Signed-off-by: stijn --- extmod/moductypes.c | 17 +++++++++-- tests/extmod/uctypes_sizeof.py | 41 ++++++++++++++++++++++++++- tests/extmod/uctypes_sizeof.py.exp | 8 ++++++ tests/extmod/uctypes_sizeof_od.py | 6 ---- tests/extmod/uctypes_sizeof_od.py.exp | 1 - 5 files changed, 63 insertions(+), 10 deletions(-) diff --git a/extmod/moductypes.c b/extmod/moductypes.c index fa743eb637..00a69a275a 100644 --- a/extmod/moductypes.c +++ b/extmod/moductypes.c @@ -143,6 +143,10 @@ static inline mp_uint_t uctypes_struct_scalar_size(int val_type) { // Get size of aggregate type descriptor static mp_uint_t uctypes_struct_agg_size(mp_obj_tuple_t *t, int layout_type, mp_uint_t *max_field_size) { + if (t->len == 0) { + syntax_error(); + } + mp_uint_t total_size = 0; mp_int_t offset_ = MP_OBJ_SMALL_INT_VALUE(t->items[0]); @@ -150,8 +154,15 @@ static mp_uint_t uctypes_struct_agg_size(mp_obj_tuple_t *t, int layout_type, mp_ switch (agg_type) { case STRUCT: + if (t->len != 2) { + syntax_error(); + } return uctypes_struct_size(t->items[1], layout_type, max_field_size); case PTR: + // Second field ignored, but should still be present for consistency. + if (t->len != 2) { + syntax_error(); + } if (sizeof(void *) > *max_field_size) { *max_field_size = sizeof(void *); } @@ -167,15 +178,17 @@ static mp_uint_t uctypes_struct_agg_size(mp_obj_tuple_t *t, int layout_type, mp_ if (item_s > *max_field_size) { *max_field_size = item_s; } - } else { + } else if (t->len == 3) { // Elements of array are aggregates item_s = uctypes_struct_size(t->items[2], layout_type, max_field_size); + } else { + syntax_error(); } return item_s * arr_sz; } default: - assert(0); + syntax_error(); } return total_size; diff --git a/tests/extmod/uctypes_sizeof.py b/tests/extmod/uctypes_sizeof.py index 6e52232e39..d295cc85b6 100644 --- a/tests/extmod/uctypes_sizeof.py +++ b/tests/extmod/uctypes_sizeof.py @@ -43,8 +43,47 @@ assert uctypes.sizeof(S.arr4) == 6 print(uctypes.sizeof(S.sub)) assert uctypes.sizeof(S.sub) == 1 -# invalid descriptor +# invalid descriptors try: print(uctypes.sizeof([])) except TypeError: print("TypeError") + +try: + print(uctypes.sizeof(())) +except TypeError: + print("TypeError") + +try: + print(uctypes.sizeof(("garbage",))) +except TypeError: + print("TypeError") + +try: + # PTR * 3 is intended to be an invalid agg_type (STRUCT, PTR, ARRAY are valid ones). + print(uctypes.sizeof((uctypes.PTR * 3,))) +except TypeError: + print("TypeError") + +try: + print(uctypes.sizeof((0, {}, "garbage"))) +except TypeError: + print("TypeError") + +try: + print(uctypes.sizeof((uctypes.PTR | 0,))) +except TypeError: + print("TypeError") + +try: + print(uctypes.sizeof((uctypes.ARRAY | 0,))) +except TypeError: + print("TypeError") + +try: + print(uctypes.sizeof((uctypes.ARRAY | 0, 1, {}, "garbage"))) +except TypeError: + print("TypeError") + +# empty descriptor +print(uctypes.sizeof({})) diff --git a/tests/extmod/uctypes_sizeof.py.exp b/tests/extmod/uctypes_sizeof.py.exp index b35b11aa0c..edcc05a441 100644 --- a/tests/extmod/uctypes_sizeof.py.exp +++ b/tests/extmod/uctypes_sizeof.py.exp @@ -5,3 +5,11 @@ TypeError 6 1 TypeError +TypeError +TypeError +TypeError +TypeError +TypeError +TypeError +TypeError +0 diff --git a/tests/extmod/uctypes_sizeof_od.py b/tests/extmod/uctypes_sizeof_od.py index 375f05f5e2..8aff363631 100644 --- a/tests/extmod/uctypes_sizeof_od.py +++ b/tests/extmod/uctypes_sizeof_od.py @@ -45,9 +45,3 @@ assert uctypes.sizeof(S.arr4) == 6 print(uctypes.sizeof(S.sub)) assert uctypes.sizeof(S.sub) == 1 - -# invalid descriptor -try: - print(uctypes.sizeof([])) -except TypeError: - print("TypeError") diff --git a/tests/extmod/uctypes_sizeof_od.py.exp b/tests/extmod/uctypes_sizeof_od.py.exp index b35b11aa0c..fb74def602 100644 --- a/tests/extmod/uctypes_sizeof_od.py.exp +++ b/tests/extmod/uctypes_sizeof_od.py.exp @@ -4,4 +4,3 @@ TypeError 6 1 -TypeError