heap-buffer-overflow: from integer overflow at mp_stream_rw #13046

junwha · 2023-11-22T06:11:02Z

Hi all, Sorry for burdening you with a lot of bug reports. we found buffer overflow at mp_stream_rw.

Summary

OS: Ubuntu 22.04
version: a00c9d5
port: unix
contribution: Junwha Hong and Wonil Jang @S2-Lab, UNIST
description:
- at mp_stream_rw py/stream.c:121 , it checks the unsigned integer size with > 0, thus it lead to integer overflow, and then heap buffer overflow

PoC

import os

os.VfsLfs1
os.VfsLfs2

class RAMBlockDevice:
    ERASE_BLOCK_SIZE = 512

    def __init__(self, blocks):
        self.data = bytearray(blocks * self.ERASE_BLOCK_SIZE)
        self.ret = 0

    def readblocks(self, block, buf, off):
        addr = block * self.ERASE_BLOCK_SIZE + off
        for i in range(len(buf)):
            buf[i] = self.data[addr + i]
        return self.ret

    def writeblocks(self, block, buf, off):
        addr = block * self.ERASE_BLOCK_SIZE + off
        for i in range(len(buf)):
            self.data[addr + i] = buf[i]
        return self.ret

    def ioctl(self, op, arg):
        if op == 4:  # block count
            return len(self.data) // self.ERASE_BLOCK_SIZE
        if op == 5:  # block size
            return self.ERASE_BLOCK_SIZE
        if op == 6:  # erase block
            return 0

def create_vfs(bdev, vfs_class):
    bdev.ret -= 0
    vfs_class.mkfs(bdev)
    vfs = vfs_class(bdev)
    with vfs.open("f", "w") as f:
        f.write("test")
    return vfs

bdev = RAMBlockDevice(8)
vfs = create_vfs(bdev, os.VfsLfs1)
f = vfs.open("f", "r")
bdev.ret = 10  # EIO
f.read(1)

Problem Statement

vstr->buf is allocated at py/stream.c:122, with sz 1-length.

STATIC mp_obj_t stream_read_generic(size_t n_args, const mp_obj_t *args, byte flags) {
        vstr_init(&vstr, sz);

The chunk vstr->buf is flown to mp_stream_rw at py/stream.c:46, as a parameter buf_ , and size is 1 here. At the first while loop py/stream.c:60, it calls io_func, which is lfs1_cache_read, and the out_sz is 10.

Problem occurs here, because the size is 1 and out_sz is 10, both are mp_uint_t, thus size -= out_sz makes integer overflow. ⇒ it’s 18446744073709551607 in unix port.

mp_uint_t mp_stream_rw(mp_obj_t stream, void *buf_, mp_uint_t size, int *errcode, byte flags) {
    byte *buf = buf_;
    typedef mp_uint_t (*io_func_t)(mp_obj_t obj, void *buf, mp_uint_t size, int *errcode);
    io_func_t io_func;
    const mp_stream_p_t *stream_p = mp_get_stream(stream);
    if (flags & MP_STREAM_RW_WRITE) {
        io_func = (io_func_t)stream_p->write;
    } else {
        io_func = stream_p->read;
    }

    *errcode = 0;
    mp_uint_t done = 0;
    while (size > 0) {
        mp_uint_t out_sz = io_func(stream, buf, size, errcode);
        // For read, out_sz == 0 means EOF. For write, it's unspecified
        // what it means, but we don't make any progress, so returning
        // is still the best option.
        if (out_sz == 0) {
            return done;
        }
        if (out_sz == MP_STREAM_ERROR) {
            // If we read something before getting EAGAIN, don't leak it
            if (mp_is_nonblocking_error(*errcode) && done != 0) {
                *errcode = 0;
            }
            return done;
        }
        if (flags & MP_STREAM_RW_ONCE) {
            return out_sz;
        }

        buf += out_sz;
        size -= out_sz;
        done += out_sz;
    }
    return done;
}

and then, because size is still over 0 (because of integer overflow), it calls io_func again, with the address of buf_ + 10 and size 18446744073709551607

then, lfs1_cache_read do memcpy with the diff, on the invalid offset buf_ + 10. thus, it is heap-over-flow.

static int lfs1_cache_read(lfs1_t *lfs1, lfs1_cache_t *rcache,
        const lfs1_cache_t *pcache, lfs1_block_t block,
        lfs1_off_t off, void *buffer, lfs1_size_t size) {
    uint8_t *data = buffer;
    LFS1_ASSERT(block < lfs1->cfg->block_count);

    while (size > 0) {
        if (pcache && block == pcache->block && off >= pcache->off &&
                off < pcache->off + lfs1->cfg->prog_size) {
            // is already in pcache?
            lfs1_size_t diff = lfs1_min(size,
                    lfs1->cfg->prog_size - (off-pcache->off));
            memcpy(data, &pcache->buffer[off-pcache->off], diff);

Patch

we need to compare out_sz and size, instead of using while (size > 0) on unsigned integer.

Crash log

#1 0x555555803061 in lfs1_cache_read /home/qbit/testing-2023/micropython/ports/unix/../../lib/littlefs/lfs1.c:39:13
#2 0x555555803061 in lfs1_file_read /home/qbit/testing-2023/micropython/ports/unix/../../lib/littlefs/lfs1.c:1611:19
#3 0x5555557d3088 in mp_vfs_lfs1_file_read /home/qbit/testing-2023/micropython/ports/unix/../../extmod/vfs_lfsx_file.c:129:28
#4 0x555555772e43 in mp_stream_rw /home/qbit/testing-2023/micropython/ports/unix/../../py/stream.c:60:28
#5 0x555555772e43 in stream_read_generic /home/qbit/testing-2023/micropython/ports/unix/../../py/stream.c:128:32
#6 0x555555782c1c in mp_execute_bytecode /home/qbit/testing-2023/micropython/ports/unix/../../py/vm.c:1042:21
#7 0x55555574261b in fun_bc_call /home/qbit/testing-2023/micropython/ports/unix/../../py/objfun.c:273:42
#8 0x555555781b2c in mp_execute_bytecode /home/qbit/testing-2023/micropython/ports/unix/../../py/vm.c:957:21
#9 0x55555574261b in fun_bc_call /home/qbit/testing-2023/micropython/ports/unix/../../py/objfun.c:273:42
#10 0x555555903f3d in execute_from_lexer /home/qbit/testing-2023/micropython/ports/unix/main.c:161:13
#11 0x555555902ad5 in do_file /home/qbit/testing-2023/micropython/ports/unix/main.c:310:12
#12 0x555555902ad5 in main_ /home/qbit/testing-2023/micropython/ports/unix/main.c:722:19
#13 0x7ffff7c29d8f in __libc_start_call_main csu/../sysdeps/nptl/libc_start_call_main.h:58:16
#14 0x7ffff7c29e3f in __libc_start_main csu/../csu/libc-start.c:392:3
#15 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! :)

The text was updated successfully, but these errors were encountered:

projectgus · 2024-01-31T07:05:05Z

Thanks for submitting this report. I am able to reproduce this, and confirm the bug exists as described. Triggering the bug depends on the user running a malformed block device driver, as provided with your PoC.

A fix should be available in MicroPython soon.

Please let us know if you plan to register a CVE for this or any other issue you've reported.

(I'd suggest in this case it's not applicable, as any exploit depends on a malformed block device driver and no such driver exists in Micropython.)

This only happens if the underlying filesystem implementation is malformed, but results in unsigned integer overflow and out of bounds read otherwise. Closes micropython#13046 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

A positive result here can result in eventual memory corruption as littlefs expects the result of a cache read/write function to be 0 or a negative integer for an error. Closes micropython#13046 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

This only happens if the underlying stream implementation is malformed, but results in unsigned integer overflow and out of bounds read otherwise. Second fix for micropython#13046 - allows for possibility an invalid result comes back from a different stream implementation. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

junwha · 2024-01-31T07:36:10Z

Thank you for confirming this issue! I sent you a DM in discord about CVEs.

A positive result here can result in eventual memory corruption as littlefs expects the result of a cache read/write function to be 0 or a negative integer for an error. Closes micropython#13046 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

This only happens if the underlying stream implementation is malformed, but results in unsigned integer overflow and out of bounds read otherwise. Second fix for micropython#13046 - allows for possibility an invalid result comes back from a different stream implementation. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

A positive result here can result in eventual memory corruption as littlefs expects the result of a cache read/write function to be 0 or a negative integer for an error. Closes micropython#13046 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>

junwha added the bug label Nov 22, 2023

projectgus mentioned this issue Jan 31, 2024

extmod: Check vfs block device invalid return values, increase error handling #13572

Merged

dpgeorge closed this as completed in 4f6d4b2 Sep 26, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

heap-buffer-overflow: from integer overflow at mp_stream_rw #13046

heap-buffer-overflow: from integer overflow at mp_stream_rw #13046

junwha commented Nov 22, 2023 •

edited

Loading

projectgus commented Jan 31, 2024 •

edited

Loading

junwha commented Jan 31, 2024

heap-buffer-overflow: from integer overflow at mp_stream_rw #13046

heap-buffer-overflow: from integer overflow at mp_stream_rw #13046

Comments

junwha commented Nov 22, 2023 • edited Loading

Summary

PoC

Problem Statement

Patch

Crash log

projectgus commented Jan 31, 2024 • edited Loading

junwha commented Jan 31, 2024

junwha commented Nov 22, 2023 •

edited

Loading

projectgus commented Jan 31, 2024 •

edited

Loading