multiple stackoverflow vulnerabilities
hongxuchen opened this issue · 3 comments
With the driver program mentioned in #54 , our fuzzing tool find multiple stackoverflow when compiled with address sanitizers.
results.txt
w01_000004,sig:6,Havoc:2071:6880,src:w02_000289.txt
w02_000009,sig:6,Havoc:11663:27520,src:w01_000262.txt
w02_000010,sig:6,Havoc:14790:27520,src:w01_000262.txt
w02_000011,sig:6,Splice:696:860,src:w01_000262.txt
w02_000039,sig:6,Splice:13:37,src:w01_000580.txt
w02_000043,sig:6,Havoc:6228:7208,src:w01_000628.txt
w02_000046,sig:6,Splice:12:24,src:w02_000635.txt
w03_000035,sig:6,Havoc:115:1075,src:w03_000380.txt
w03_000037,sig:6,Splice:4:16,src:w00_000040.txt
w03_000052,sig:6,Havoc:373:409,src:w02_000516.txt
w03_000053,sig:6,Havoc:7:291,src:w03_000380.txt
w03_000056,sig:6,Splice:39:39,src:w04_000542.txt
w03_000057,sig:6,Havoc:268:307,src:w01_000518.txt
w03_000060,sig:6,Havoc:33:176,src:w02_000563.txt
w04_000015,sig:6,Havoc:14076:22272,src:w03_000188.txt
w04_000016,sig:6,Havoc:4230:8352,src:w04_000306.txt
w04_000017,sig:6,Havoc:13474:16704,src:w04_000306.txt
w04_000026,sig:6,Splice:41:92,src:w03_000381.txt
w04_000032,sig:6,Havoc:48:942,src:w02_000193.txt
w04_000042,sig:6,Havoc:95:1679,src:w04_000569.txt
w04_000051,sig:6,Splice:19:32,src:w03_000525.txt
results.txt contains the error messages and other files are test inputs. These test inputs do not cause segfaults without address sanitizers and have some duplicates.
gdb backtrace on the compiled binary reveals that these stack overflows involve pnext, parse_mul_div_rem, parse_shifts, parse_plus_minus, parse_bitwise_and, parse_bitwise_or, parse_logical_and, parse_call_dot_mem, etc. We will try to figure out the actual root.
Dear @cpq and @dimonomid, we just noticed that the applied fix for this type of error is basically doing another parsing depth (STACK_LIMIT / BINOP_STACK_FRAME_SIZE) checking. But it leads to some limitation, e.g., it cannot deal with those sane javascript inputs that exceed this value when parsed by mjs, right?
Yeah, that's right; that's the tradeoff we had to make for now. It allows for ~50 nesting level, which doesn't seem too strict given the fact that primary targets for mJS are microcontrollers with all sorts of limited resources.
If you ask me, I personally believe that the parser should have been implemented using dramatically different approach: using a parser generator like lemon, so that the C stack consumption does not depend on JS stack consumption. We didn't come to agreement with other team members in that respect, so right now it's implemented as a manually-written recursive descent parser.
@dimonomid That's not a big issue from our fuzzing tool's perspective, however our (shallow) differential based static analysis frontend tool locates them and in my opinion that part can be improved. Thanks anyway.