search

ccxvii / mujs

An embeddable Javascript interpreter in C.
http://mujs.com/
ISC License
794 stars 96 forks source link

BUG #166

Closed vraebfdsb closed 1 year ago

vraebfdsb commented 1 year ago

Hi, developers of mujs: When I tested mujs, I found multiple bugs.

Step To Reproduce

./mujs $POC

Environment

BUG1

(base) ./mujs ./POS1
=================================================================
==94510==ERROR: AddressSanitizer: heap-use-after-free on address 0x603000001dc0 at pc 0x55d5f13ee208 bp 0x7ffe207cce20 sp 0x7ffe207cc5d0
READ of size 1 at 0x603000001dc0 thread T0
    #0 0x55d5f13ee207 in strcmp (/AFLplusplus/aflplusplus_pro/version_10_17/programs/unibench_asan/mujs+0x55207) (BuildId: 00fce144ce9a724e520eb500f9f90b062f56aea3)
    #1 0x55d5f1520f5e in lookup /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsproperty.c:50:11
    #2 0x55d5f1520f5e in jsV_getproperty /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsproperty.c:189:22
    #3 0x55d5f1520f5e in jsV_nextiterator /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsproperty.c:332:7
    #4 0x55d5f1600bfa in jsR_run /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1737:11
    #5 0x55d5f1543607 in jsR_callscript /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1216:2
    #6 0x55d5f1543607 in js_call /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1274:3
    #7 0x55d5f155090d in js_dofile /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsstate.c:254:2
    #8 0x55d5f1619762 in main /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/main.c:363:7
    #9 0x7f55bf0e7d8f  (/lib/x86_64-linux-gnu/libc.so.6+0x29d8f) (BuildId: 89c3cb85f9e55046776471fed05ec441581d1969)
    #10 0x7f55bf0e7e3f in __libc_start_main (/lib/x86_64-linux-gnu/libc.so.6+0x29e3f) (BuildId: 89c3cb85f9e55046776471fed05ec441581d1969)
    #11 0x55d5f13d7574 in _start (/AFLplusplus/aflplusplus_pro/version_10_17/programs/unibench_asan/mujs+0x3e574) (BuildId: 00fce144ce9a724e520eb500f9f90b062f56aea3)

0x603000001dc0 is located 16 bytes inside of 32-byte region [0x603000001db0,0x603000001dd0)
freed by thread T0 here:
    #0 0x55d5f145a112 in free (/AFLplusplus/aflplusplus_pro/version_10_17/programs/unibench_asan/mujs+0xc1112) (BuildId: 00fce144ce9a724e520eb500f9f90b062f56aea3)
    #1 0x55d5f15520aa in js_defaultalloc /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsstate.c:24:3

previously allocated by thread T0 here:
    #0 0x55d5f145a7e6 in __interceptor_realloc (/AFLplusplus/aflplusplus_pro/version_10_17/programs/unibench_asan/mujs+0xc17e6) (BuildId: 00fce144ce9a724e520eb500f9f90b062f56aea3)
    #1 0x55d5f151e8b0 in js_malloc /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:44:14
    #2 0x55d5f151e8b0 in itnewnodeix /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsproperty.c:239:22
    #3 0x55d5f151e8b0 in jsV_newiterator /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsproperty.c:292:25
    #4 0x55d5f1603715 in jsR_run /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1728:11
    #5 0x55d5f1543607 in jsR_callscript /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1216:2
    #6 0x55d5f1543607 in js_call /AFLplusplus/kbench_aflplusplus_images/unibench_2022_10_13_version/unibench/mujs/./jsrun.c:1274:3

SUMMARY: AddressSanitizer: heap-use-after-free (/AFLplusplus/aflplusplus_pro/version_10_17/programs/unibench_asan/mujs+0x55207) (BuildId: 00fce144ce9a724e520eb500f9f90b062f56aea3) in strcmp

POC

mujs_POS.zip

avih commented 1 year ago

I think it should be obvious that the fix - c907989 broke the public mujs API, and code which uses js_nextiterator with two arguments now fails to compile.

I think such breakage at the very least deserves some more wording (e.g. at the commit message) and docs of the new argument.

Preferably though, the breakage should be reverted, and maybe add js_nextiterator2 with different signature which addresses the issue (and documented, and allows feature detection via preprocessor macro).

ccxvii commented 1 year ago

Thanks for the report. I've pushed a (revised to not change the API) fix.