Closed
Description
Thank you for the super fast responses for the previous bugs, we found new bug in the core module.
Summary
- OS: Ubuntu 22.04
- version: micropython a00c9d5
- port: unix
- contribution: Junwha Hong and Wonil Jang @S2-Lab, UNIST
- description: Because the length check of int_to_bytes does not include 0, it leads to heap-buffer-overflow and Segmentation fault
PoC
b = bytes(range(20))
ib = int.from_bytes(b, "big")
print(ib.to_bytes( 0, "big"))Expected behavior in python3
python3 ./poc_to_bytes
Traceback (most recent call last):
File "/home/qbit/testing-2023/micropython/fuzzing/output/default/crashes/./poc_to_bytes", line 3, in <module>
print(ib.to_bytes( 0, "big"))
OverflowError: int too big to convertmicropython
Segmentation faultProblem Statement
For the PoC, at py/objint.c:423, the len is 0, thus it pass through if (len < 0)
STATIC mp_obj_t int_to_bytes(size_t n_args, const mp_obj_t *args) {
// TODO: Support signed param (assumes signed=False)
(void)n_args;
mp_int_t len = mp_obj_get_int(args[1]);
if (len < 0) {
mp_raise_ValueError(NULL);
}
bool big_endian = args[2] != MP_OBJ_NEW_QSTR(MP_QSTR_little);
vstr_t vstr;
vstr_init_len(&vstr, len);
byte *data = (byte *)vstr.buf;
memset(data, 0, len);
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
if (!mp_obj_is_small_int(args[0])) {
mp_obj_int_to_bytes_impl(args[0], big_endian, len, data);Then, it calls mpz_as_bytes at with zero len. here, thus b and buf are same before the for loop.
void mpz_as_bytes(const mpz_t *z, bool big_endian, size_t len, byte *buf) {
byte *b = buf;
if (big_endian) {
b += len;
}
mpz_dig_t *zdig = z->dig;
int bits = 0;
mpz_dbl_dig_t d = 0;
mpz_dbl_dig_t carry = 1;
for (size_t zlen = z->len; zlen > 0; --zlen) {
bits += DIG_SIZE;
d = (d << DIG_SIZE) | *zdig++;
for (; bits >= 8; bits -= 8, d >>= 8) {
mpz_dig_t val = d;
if (z->neg) {
val = (~val & 0xff) + carry;
carry = val >> 8;
}
if (big_endian) {
*--b = val;
if (b == buf) {
return;
}
} else {
...
}
}
}
// fill remainder of buf with zero/sign extension of the integer
if (big_endian) {
len = b - buf;
} else {
...
}
memset(buf, z->neg ? 0xff : 0x00, len);Because it never satisfy the condition if (b == buf), the b overflows to the left object. (under the start of the pointer).
Additionally, the length is -20, because unexpectedly the condition b < buf was satisfied.
Patch
Patch is very simple. we just need to include 0 to the case.
STATIC mp_obj_t int_to_bytes(size_t n_args, const mp_obj_t *args) {
// TODO: Support signed param (assumes signed=False)
(void)n_args;
mp_int_t len = mp_obj_get_int(args[1]);
if (len <= 0) {
mp_raise_ValueError(NULL);
}
bool big_endian = args[2] != MP_OBJ_NEW_QSTR(MP_QSTR_little);
vstr_t vstr;
vstr_init_len(&vstr, len);
byte *data = (byte *)vstr.buf;
memset(data, 0, len);
#if MICROPY_LONGINT_IMPL != MICROPY_LONGINT_IMPL_NONE
if (!mp_obj_is_small_int(args[0])) {
**mp_obj_int_to_bytes_impl(args[0], big_endian, len, data);**Crash log
#0 0x5555556ae6e0 in mpz_as_bytes /home/qbit/testing-2023/micropython/ports/unix/../../py/mpz.c:1611:22
#1 0x55555574583a in int_to_bytes /home/qbit/testing-2023/micropython/ports/unix/../../py/objint.c:440:9
#2 0x555555782c1c in mp_execute_bytecode /home/qbit/testing-2023/micropython/ports/unix/../../py/vm.c:1042:21
#3 0x55555574261b in fun_bc_call /home/qbit/testing-2023/micropython/ports/unix/../../py/objfun.c:273:42
#4 0x555555903f3d in execute_from_lexer /home/qbit/testing-2023/micropython/ports/unix/main.c:161:13
****#5 0x555555902ad5 in do_file /home/qbit/testing-2023/micropython/ports/unix/main.c:310:12
#6 0x555555902ad5 in main_ /home/qbit/testing-2023/micropython/ports/unix/main.c:722:19
#7 0x7ffff7c29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#8 0x7ffff7c29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#9 0x555555593a34 in _start (/home/qbit/testing-2023/micropython/ports/unix/build-standard/micropython+0x3fa34)Thank you for taking the time to review our bug report! :)