Function r_parse_parse Lacks Boundary Check

[mmmdzz]

For x86.pseudo (and other RParsePlugins), function replace lacks a boundary check for newstr, leading heap-/stack-based buffer over-write. So do other RParsePlugins.

Such buffer overflow could be trigger from rasm2. As r_parse_parse, which actually calls function replace for x86.pseudo, could extend buf_asm and write back to buf_asm, it would cause a heap-based buffer over-write.

Work environment

Questions	Answers
OS/arch/bits (mandatory)	Ubuntu x86 64
File format of the file you reverse (mandatory)	ELF
Architecture/bits of the file (mandatory)	x86/64
r2 -v full output, not truncated (mandatory)	radare2 3.6.0-git 22043 @ linux-x86-64 git.3.5.1-235-ga1ade7d87 commit: a1ade7d build: 2019-06-15__00:21:43

Expected behavior

NO CRASH

Actual behavior

$ rasm2 -d -a x86 -b 64 -F :x86.pseudo a6
while (CX != 0) { var = *(DS*16 + SI) - *(ES*16 + DI); SI++; DI++; CX--; if (!var) break; }
double free or corruption (out)
Aborted (core dumped)

Steps to reproduce the behavior

• Follow actions mentioned above.

Additional Logs, screenshots, source-code, configuration dump, ...

Output of valgrind: valgrind.txt

[mmmdzz]

Similar error for avr

$ rasm2 -d -a avr -F :avr.pseudo 1294
Segmentation fault

Other RParsePlugins also lack boundary checking.

[radare]

Yeah i have half written the plans to fully rewrite the rparse api to get 2-3 args instead of 10 and be safer using rstrbuf or something that checks bounds. A quickfix should come before that refactoring (as well as some more tests)

…

On 15 Jun 2019, at 06:49, mmmdzz ***@***.***> wrote: For x86.pseudo (and other RParsePlugins), function parse lacks a boundary check for newstr, leading heap-/stack-based buffer over-write. Such buffer overflow could be trigger from rasm2. As r_parse_parse, which actually calls function parse for x86.pseudo, could extend buf_asm and write back to buf_asm, it would cause a heap-based buffer over-write. Work environment Questions Answers OS/arch/bits (mandatory) Ubuntu x86 64 File format of the file you reverse (mandatory) ELF Architecture/bits of the file (mandatory) x86/64 r2 -v full output, not truncated (mandatory) radare2 3.6.0-git 22043 @ linux-x86-64 git.3.5.1-235-ga1ade7d87 commit: a1ade7d build: 2019-06-15__00:21:43 Expected behavior NO CRASH Actual behavior $ rasm2 -d -a x86 -b 64 -F :x86.pseudo a6 while (CX != 0) { var = *(DS*16 + SI) - *(ES*16 + DI); SI++; DI++; CX--; if (!var) break; } double free or corruption (out) Aborted (core dumped) Steps to reproduce the behavior Follow actions mentioned above. Additional Logs, screenshots, source-code, configuration dump, ... Output of valgrind: valgrind.txt — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[radare]

fixed, rbufstr overflows are tricky to fix, i created a wrapper function that handles bigger buffers for such cases, but the proper fix should be by redesigning RParse api. i will work on that on that after the release