lometsj / blog_repo

blog at issue
0 stars 0 forks source link

erofs-utils heap-based overflow when extract a file system image via fsck.erofs #1

Open lometsj opened 1 year ago

lometsj commented 1 year ago

heap-base overflow of erofs-utils

project

https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git

env

tested on fedora 37

version

erofs-utils v1.6

reproduce

➜  erofs-utils ./fsck/fsck.erofs -V
fsck.erofs 1.6
➜  erofs-utils ./fsck/fsck.erofs --extract=/tmp/test_out ../../erofs-utils-debug/output_fsck2/default:id:000011,sig:06,src:000303,time:17951196,execs:70778046,op:havoc,rep:16 
<E> erofs_io: Reach EOF of device - ../../erofs-utils-debug/output_fsck2/default:id:000011,sig:06,src:000303,time:17951196,execs:70778046,op:havoc,rep:16:[4124, 15388867821540].
free(): invalid next size (normal)
[1]    266474 IOT instruction (core dumped)  ./fsck/fsck.erofs --extract=/tmp/test_out 

source code analysis

in erofs_read_one_data function at data.c, it call dev_read to read from buf

gef➤  l erofs_read_one_data
162                             size_t len)
163     {
164             struct erofs_map_dev mdev;
165             int ret;
166
167             mdev = (struct erofs_map_dev) {
168                     .m_deviceid = map->m_deviceid,
169                     .m_pa = map->m_pa,
170             };
171             ret = erofs_map_dev(&sbi, &mdev);
gef➤  
172             if (ret)
173                     return ret;
174
175             ret = dev_read(mdev.m_deviceid, buffer, mdev.m_pa + offset, len);
176             if (ret < 0)
177                     return -EIO;
178             return 0;
179     }

and the var len is depends on map->m_plen, and map->m_plen depends on inode->i_size which import from image file without check, lead to heap overflow

the top chunk is override as below

gef➤  heap chunks
Chunk(addr=0x40e010, size=0x290, flags=PREV_INUSE)
    [0x000000000040e010     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................]
Chunk(addr=0x40e2a0, size=0x1010, flags=PREV_INUSE)
    [0x000000000040e2a0     2f 74 6d 70 2f 74 65 73 74 5f 6f 75 74 00 00 00    /tmp/test_out...]
Chunk(addr=0x40f2b0, size=0x80, flags=PREV_INUSE)
    [0x000000000040f2b0     2e 2e 2f 2e 2e 2f 65 72 6f 66 73 2d 75 74 69 6c    ../../erofs-util]
Chunk(addr=0x40f330, size=0x1010, flags=PREV_INUSE)
    [0x000000000040f330     00 00 00 00 00 00 00 00 00 00 00 00 00 00 00 00    ................]
Chunk(addr=0x410340, size=0x697373696d736e60, flags=PREV_INUSE)
    [0x0000000000410340     6f 6e 20 61 6e 64 20 6d 65 6d 6f 72 00 00 00 00    on and memor....]
Chunk(addr=0x410340, size=0x697373696d736e60, flags=PREV_INUSE)  ←  top chunk
gef➤  

default_id_000011,sig_06,src_000303,time_17951196,execs_70778046,op_havoc,rep_16.zip

attritionorg commented 1 year ago

Also, generally don't request a CVE ID for every finding via fuzzing as they are prone to false positives in one manner or another. You triggered an OOB whatever? Great! Is it in the test suite or CLI utils with no real vector? Not so great. Give time for the developers of the project to confirm it is a valid issue before requesting.

I know I know, for the rare ones in the peanut gallery that say "but wasn't the original purpose of CVE to provide an ID while that is figured out?" Yep! But also, back then? They were quick to REJECT an ID and add a meaningful description update to an entry which is like getting a FFRDC to look past the price-gouged bill that tax-payers have to fund. It just doesn't happen much these days.

lometsj commented 1 year ago

When you fuzz things and get CVEs for the issues you find, please also report to upstream in an accessible way to CVE consumers (ie, reference them in the CVE). You generally do more harm than good when you publicize security issues in this way without doing the minimum to get the issues fixed.

Also, generally don't request a CVE ID for every finding via fuzzing as they are prone to false positives in one manner or another. You triggered an OOB whatever? Great! Is it in the test suite or CLI utils with no real vector? Not so great. Give time for the developers of the project to confirm it is a valid issue before requesting.

I know I know, for the rare ones in the peanut gallery that say "but wasn't the original purpose of CVE to provide an ID while that is figured out?" Yep! But also, back then? They were quick to REJECT an ID and add a meaningful description update to an entry which is like getting a FFRDC to look past the price-gouged bill that tax-payers have to fund. It just doesn't happen much these days.

I do commit bug report to maintainer or developer before publicizing it and they do confirm it. I also try my best to do source code analysis to help maintainer to figure it. I don't know how to request a CVE id without a reference of bug report, so I create this issue. I admit I created this issue before repairing it, it's my fault. I know you guys hate fuzzing police. I'm sorry to make you think I'm.

hsiangkao commented 1 year ago

When you fuzz things and get CVEs for the issues you find, please also report to upstream in an accessible way to CVE consumers (ie, reference them in the CVE). You generally do more harm than good when you publicize security issues in this way without doing the minimum to get the issues fixed.

Also, generally don't request a CVE ID for every finding via fuzzing as they are prone to false positives in one manner or another. You triggered an OOB whatever? Great! Is it in the test suite or CLI utils with no real vector? Not so great. Give time for the developers of the project to confirm it is a valid issue before requesting. I know I know, for the rare ones in the peanut gallery that say "but wasn't the original purpose of CVE to provide an ID while that is figured out?" Yep! But also, back then? They were quick to REJECT an ID and add a meaningful description update to an entry which is like getting a FFRDC to look past the price-gouged bill that tax-payers have to fund. It just doesn't happen much these days.

I do commit bug report to maintainer or developer before publicizing it and they do confirm it. I also try my best to do source code analysis to help maintainer to figure it.

The problem was that you guys told me that you'd like to fix them upstream as well, yet I don't find that happen in time until these two CVEs unvealed so I have to fix them urgently upstream although these are all about malicious images.

I know you guys hate fuzzing police. I'm sorry to make you think I'm.

Fuzzing is always a good attempt, yet I also don't think each fuzzing issue needs to raise an individual CVE (If you pay more time fuzzing other stuffs I think that is also the case). Compared with your reports, how about contributing a fuzzing framework to the opensource community as well? Personally I have quite limited time on all stuffs if someone else could help on that that would be much better.

hsiangkao commented 1 year ago

https://git.kernel.org/pub/scm/linux/kernel/git/xiang/erofs-utils.git/patch/?id=2145dff03dd3f3f74bcda3b52160fbad37f7fcfe The proposed fix for this issue.