Integer overflow in realloc call in radareorg/radare2

Valid

Reported on

Nov 26th 2022


Description

Integer overflow in realloc and memcpy calls in core_anal_graph_label. In the process of concatenating source lines based on DWARF data, the resulting size (32bit signed int) can overflow. The sizes of the realloc and memcpy calls differ, and potentially can lead to writes in an unintended location.

Proof of Concept

poc

build the poc with make or download from releases.

then run:

r2 -A intof_mod

and in r2 shell:

s main
agl

note: it's important to run intof_mod in the same directory as dummy.c, as the DWARF data relies on it.

Asan

potato@dev-ubuntu22:~/projects/sec/poc/r2/r2_int_of$ r2 -A intof_mod                                                                                                                   
WARN: run r2 with -e bin.cache=true to fix relocations in disassembly
INFO: Analyze all flags starting with sym. and entry0 (aa)
INFO: Analyze all functions arguments/locals (afva@@@F)
INFO: Analyze function calls (aac)
INFO: Analyze len bytes of instructions for references (aar)
INFO: Finding and parsing C++ vtables (avrr)
INFO: Type matching analysis for all functions (aaft)
INFO: Propagate noreturn information (aanr)
INFO: Integrate dwarf function information
INFO: Use -AA or aaaa to perform additional experimental analysis
 -- Set 'e bin.dbginfo=true' to load debug information at startup.
[0x00001060]> s main
[0x00001149]> agl
canal.c:1393:43: runtime error: signed integer overflow: 2147483646 + 8 cannot be represented in type 'int'
=================================================================
==84280==ERROR: AddressSanitizer: requested allocation size 0xffffffff80000006 (0xffffffff80001008 after adjustments for alignment, red zones etc.) exceeds maximum supported size of 0x10000000000 (thread T0)
    #0 0x7fed1f848c18 in __interceptor_realloc ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164
    #1 0x7fed1b9cbbad in core_anal_graph_label /home/potato/stuff/repos/radare2/libr/core/canal.c:1393
    #2 0x7fed1b9d1153 in core_anal_graph_construct_nodes /home/potato/stuff/repos/radare2/libr/core/canal.c:1668
    #3 0x7fed1b9d443a in core_anal_graph_nodes /home/potato/stuff/repos/radare2/libr/core/canal.c:1873
    #4 0x7fed1b9ef175 in r_core_anal_graph /home/potato/stuff/repos/radare2/libr/core/canal.c:3796
    #5 0x7fed1b6868eb in cmd_anal_graph /home/potato/stuff/repos/radare2/libr/core/cmd_anal.c:10926
    #6 0x7fed1b69f966 in cmd_anal /home/potato/stuff/repos/radare2/libr/core/cmd_anal.c:12878
    #7 0x7fed1b9a716a in r_cmd_call /home/potato/stuff/repos/radare2/libr/core/cmd_api.c:519
    #8 0x7fed1b8077a8 in r_core_cmd_subst_i /home/potato/stuff/repos/radare2/libr/core/cmd.c:4722
    #9 0x7fed1b7f5ff6 in r_core_cmd_subst /home/potato/stuff/repos/radare2/libr/core/cmd.c:3558
    #10 0x7fed1b814844 in run_cmd_depth /home/potato/stuff/repos/radare2/libr/core/cmd.c:5621
    #11 0x7fed1b815a19 in r_core_cmd /home/potato/stuff/repos/radare2/libr/core/cmd.c:5705
    #12 0x7fed1b4c0b07 in r_core_prompt_exec /home/potato/stuff/repos/radare2/libr/core/core.c:3570
    #13 0x7fed1b4be0b8 in r_core_prompt_loop /home/potato/stuff/repos/radare2/libr/core/core.c:3388
    #14 0x7fed1e618e8a in r_main_radare2 /home/potato/stuff/repos/radare2/libr/main/radare2.c:1666
    #15 0x55bd629d5af2 in main /home/potato/stuff/repos/radare2/binr/radare2/radare2.c:104
    #16 0x7fed1dd04d8f in __libc_start_call_main ../sysdeps/nptl/libc_start_call_main.h:58

==84280==HINT: if you don't care about these errors you may set allocator_may_return_null=1
SUMMARY: AddressSanitizer: allocation-size-too-big ../../../../src/libsanitizer/asan/asan_malloc_linux.cpp:164 in __interceptor_realloc
==84280==ABORTING

Impact

Unintended memory access and availability.

We are processing your report and will contact the radareorg/radare2 team within 24 hours. a month ago
We have contacted a member of the radareorg/radare2 team and are waiting to hear back a month ago
pancake
a month ago

argh! totally missed the report until now! I have fixed the issue in https://github.com/radareorg/radare2/commit/b53a1583d05c3a5bfe5fa60da133fe59dfbb02b8 it's a blind fix, because i compiled your reproducer but the resulting binary doesn't crash at all. i had to tweak the code to make it explode.

To summarize:

  • i'm so happy to see more reports coming from huntrdev!
  • So sorry for violating the 1 day fix rule, i totally missed the mail notification :sad:
  • I recognize the bug as valid
  • I pushed a blindfix, so i would appreciate testing it again on latest master
  • this bug should be reproducible on older versions of r2
  • I would like to have a compiled reproducer to ship it in the radare2-testbins repository (can you make a PR in there?) and add a test in r2/test/db/.. ?
  • The asan crashlog was enough for me to understand the bug and push a fix.

Thanks!

solid-snail
a month ago

Researcher


no problem, it's always nice being welcome!

I've added a comment to your commit, I'll copy it here for convenience:

I think theoretically flen could already be overflowed at that point. If I recall correctly, when I tried testing with source line of that length (2~4 GB) I had crashes earlier on in the startup process, so I couldn't validate it. Might be worth using a different type for flen and idx (the 64bit unsigned counterpart), using pointers to track the locations, or limiting line length (or checking for overflow) in the line slurping process.

And regarding the compiled reproducer, I have uploaded one to the releases in repo I've linked, could you validate it works before I open a PR? intof_mod dummy.c

I think both files need to be in the same directory in order for it to work. It might also fail on the difference in the directory path in the DWARF data, so let me know.

pancake
a month ago

Uhm, can't repro, so I guess I fixed it. Can you confirm? i'll close the ticket as aproved now 👍

solid-snail
a month ago

Researcher


Can confirm that I hit the integer overflow warning, and it broke out of the loop in debugging 👌

pancake
a month ago

So, let's say that my blind fix worked :) kudos!

solid-snail
a month ago

Researcher


Was about to open the PR for radare2-testbins, the checklist mentions documentation. I don't see any obvious place for it in the repo. Where do you want me to add it?

Sorry for the delay, and nice job on the fix btw!

solid-snail
a month ago

Researcher


hey :) is there anything else blocking the review?

pancake validated this vulnerability a month ago

I can't say if this crash could lead to code execution, so i'm taking the default severity from the CWE-190 (intovf). Please let me know if there's anything left. and sorry for the delay :)

solid-snail has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
solid-snail
a month ago

Researcher


yeah, it wouldn't have been trivial to get code execution. The only way I could see it happening is if you wrapped around back to 0-7, and then memcpy would overflow the buffer (because +8 is added to the value given to realloc). But in my attempts it wasn't trivial to get.

solid-snail
a month ago

Researcher


Sorry to bug you again, but is there anything holding back the disclosure? Right now it's on 'awaiting fix' status, and I believe you already did fix it

pancake marked this as fixed in 5.8.0 with commit b53a15 a month ago
pancake has been awarded the fix bounty
This vulnerability has been assigned a CVE
pancake published this vulnerability a month ago
pancake
a month ago

Oops, sorry again :sad: it should be good now, huntr added an extra step before closing the ticket and i didnt payed attention to it. It should be good to go now thanks for the headsup!

to join this conversation