8000 py/modstruct: Check and prevent buffer-write overflow in struct packing. · micropython/micropython-esp32@2daacc5 · GitHub
[go: up one dir, main page]
Skip to content
This repository was archived by the owner on Sep 6, 2023. It is now read-only.

Commit 2daacc5

Browse files
dpgeorgedpgeorge
committed
py/modstruct: Check and prevent buffer-write overflow in struct packing.
Prior to this patch, the size of the buffer given to pack_into() was checked for being too small by using the count of the arguments, not their actual size. For example, a format spec of '4I' would only check that there was 4 bytes available, not 16; and 'I' would check for 1 byte, not 4. The pack() function is ok because its buffer is created to be exactly the correct size. The fix in this patch calculates the total size of the format spec at the start of pack_into() and verifies that the buffer is large enough. This adds some computational overhead, to iterate through the whole format spec. The alternative is to check during the packing, but that requires extra code to handle alignment, and the check is anyway not needed for pack(). So to maintain minimal code size the check is done using struct_calcsize.
1 parent 79d5acb commit 2daacc5

File tree

3 files changed

+26
-13
lines changed

3 files changed

+26
-13
lines changed

‎py/modstruct.c

Lines changed: 16 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -174,37 +174,35 @@ STATIC mp_obj_t struct_unpack_from(size_t n_args, const mp_obj_t *args) {
174174
}
175175
MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(struct_unpack_from_obj, 2, 3, struct_unpack_from);
176176

177-
STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, byte* end_p, size_t n_args, const mp_obj_t *args) {
177+
// This function assumes there is enough room in p to store all the values
178+
STATIC void struct_pack_into_internal(mp_obj_t fmt_in, byte *p, size_t n_args, const mp_obj_t *args) {
178179
const char *fmt = mp_obj_str_get_str(fmt_in);
179180
char fmt_type = get_fmt_type(&fmt);
180181

181182
size_t i;
182183
for (i = 0; i < n_args;) {
183-
mp_uint_t sz = 1;
184+
mp_uint_t cnt = 1;
184185
if (*fmt == '\0') {
185186
// more arguments given than used by format string; CPython raises struct.error here
186187
break;
187188
}
188189
if (unichar_isdigit(*fmt)) {
189-
sz = get_fmt_num(&fmt);
190-
}
191-
if (p + sz > end_p) {
192-
mp_raise_ValueError("buffer too small");
190+
cnt = get_fmt_num(&fmt);
193191
}
194192

195193
if (*fmt == 's') {
196194
mp_buffer_info_t bufinfo;
197195
mp_get_buffer_raise(args[i++], &bufinfo, MP_BUFFER_READ);
198-
mp_uint_t to_copy = sz;
196+
mp_uint_t to_copy = cnt;
199197
if (bufinfo.len < to_copy) {
200198
to_copy = bufinfo.len;
201199
}
202200
memcpy(p, bufinfo.buf, to_copy);
203-
memset(p + to_copy, 0, sz - to_copy);
204-
p += sz;
201+
memset(p + to_copy, 0, cnt - to_copy);
202+
p += cnt;
205203
} else {
206204
// If we run out of args then we just finish; CPython would raise struct.error
207-
while (sz-- && i < n_args) {
205+
while (cnt-- && i < n_args) {
208206
mp_binary_set_val(fmt_type, *fmt, args[i++], &p);
209207
}
210208
}
@@ -219,8 +217,7 @@ STATIC mp_obj_t struct_pack(size_t n_args, const mp_obj_t *args) {
219217
vstr_init_len(&vstr, size);
220218
byte *p = (byte*)vstr.buf;
221219
memset(p, 0, size);
222-
byte *end_p = &p[size];
223-
struct_pack_into_internal(args[0], p, end_p, n_args - 1, &args[1]);
220+
struct_pack_into_internal(args[0], p, n_args - 1, &args[1]);
224221
return mp_obj_new_str_from_vstr(&mp_type_bytes, &vstr);
225222
}
226223
MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(struct_pack_obj, 1, MP_OBJ_FUN_ARGS_MAX, struct_pack);
@@ -240,7 +237,13 @@ STATIC mp_obj_t struct_pack_into(size_t n_args, const mp_obj_t *args) {
240237
byte *end_p = &p[bufinfo.len];
241238
p += offset;
242239

243-
struct_pack_into_internal(args[0], p, end_p, n_args - 3, &args[3]);
240+
// Check that the output buffer is big enough to hold all the values
241+
mp_int_t sz = MP_OBJ_SMALL_INT_VALUE(struct_calcsize(args[0]));
242+
if (p + sz > end_p) {
243+
mp_raise_ValueError("buffer too small");
244+
}
245+
246+
struct_pack_into_internal(args[0], p, n_args - 3, &args[3]);
244247
return mp_const_none;
245248
}
246249
MP_DEFINE_CONST_FUN_OBJ_VAR_BETWEEN(struct_pack_into_obj, 3, MP_OBJ_FUN_ARGS_MAX, struct_pack_into);

‎tests/basics/struct1.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,12 @@
6969
struct.pack_into('<bbb', buf, -6, 0x44, 0x45, 0x46)
7070
print(buf)
7171

72+
# check that we get an error if the buffer is too small
73+
try:
74+
struct.pack_into('I', bytearray(1), 0, 0)
75+
except:
76+
print('struct.error')
77+
7278
try:
7379
struct.pack_into('<bbb', buf, 7, 0x41, 0x42, 0x43)
7480
except:

‎tests/basics/struct2.py

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,10 @@
3030
struct.unpack('2H', b'\x00\x00')
3131
except:
3232
print('Exception')
33+
try:
34+
struct.pack_into('2I', bytearray(4), 0, 0)
35+
except:
36+
print('Exception')
3337

3438
# check that unknown types raise an exception
3539
try:

0 commit comments

Comments
 (0)
0