radareorg / r2ghidra

Native Ghidra Decompiler for r2
https://www.radare.org/
GNU Lesser General Public License v3.0
340 stars 43 forks source link

Problem regarding r2ghidra installation on Windows #141

Open MateiTiplea opened 3 months ago

MateiTiplea commented 3 months ago

Hello,

I am having an issue regarding the installation of the plugin on Windows 11 x64 machines. I downloaded the latest release of radare2 from the github for w64 and extracted the binaries on the system. The tool worked as expected when I tried it on a sample binary file.

Then I tried installing the r2ghidra plugin using the latest pre-compiled dll for w64 and, as instructed in the readme of the plugin, I ran the command r2 -hh in order to find the R2_USER_PLUGINS (output of the command provided in screenshot). image

I then copied the dll to C:\Users\<user>\.local\share\radare2\plugins. image

But now, when I try to use the radare2 tool, I get the following error. image

Note: I tried installing the plugin using the r2pm also, but the command did not work. image

Please help! Thank you!

AlexandruGabriel02 commented 3 months ago

I also get the same issue on Windows 10

Adde2130 commented 2 months ago

Is there still no workaround/solution to this? R2Ghidra has been broken a good month now on windows

trufae commented 2 months ago

I am the only one maintaining the whole r2 and didnt had a chance to fire up a windows yet. It’s in my infinite todo list but if anyone had a chance to take a look i would appreciate it

Adde2130 commented 2 months ago

Ok, I've figured out the problem, but as for a fix I only got a workaround (I've never worked on public repos before so I apologize).

https://github.com/radareorg/r2ghidra/blob/a167eaf91e939dd0e2114698eb75945e272ac6a8/src/SleighAsm.cpp#L403-L463

This function is the culprit. First of all, if you are on Windows and have compiled Radare2 yourself, you need to manually set a SLEIGHHOME environment variable since none of the folders checked in this function will exist. Second, both of the free calls at line 422 and line 441 cause a crash. Why? I didn't have time to figure out (again, sorry).

https://github.com/radareorg/r2ghidra/blob/a167eaf91e939dd0e2114698eb75945e272ac6a8/src/SleighAsm.cpp#L422

https://github.com/radareorg/r2ghidra/blob/a167eaf91e939dd0e2114698eb75945e272ac6a8/src/SleighAsm.cpp#L441

Since the second free is only reached if SLEIGHHOME isn't set, you only have to care about the first one. I couldn't be bothered recompiling it so I just filled the free call bytes at offset 0x509f3 with NOPs in core_r2ghidra.dll. I don't know if this causes a memory leak but at least it works now, so hopefully this helps out somewhat at least

trufae commented 2 months ago

r2ai helped here:

[r2ai:0x100003a84]> !cat /tmp/a.txt

std::string foo() {
    char *path = strdup ("/bin/ls");
    // const char *path = "/bin/ls";
    std::string res (path);
    // free (path);
    return res;
}

[r2ai:0x100003a84]> -i /tmp/a.txt is std::string messing with the heaped value passed as argument that can cause a double free only on windows? [r2ai] Using GPU

A classic C++ gotcha!

Yes, you are correct. The std::string constructor is taking a copy of the char* pointer passed to it, and then the original pointer is being freed in the foo() function. This can cause a double-free error on Windows (and possibly other platforms that use the same heap implementation).

The reason for this is that std::string uses a technique called "slicing" when creating a copy of its contents from a C-style string. When you pass a char* pointer to the std::string constructor, it creates a temporary std::string object and copies the contents of the original array into it. However, this temporary object is not stored anywhere, so when the foo() function returns, the original char* pointer is still pointing to the same memory location that was just freed.

On Windows, the heap implementation used by Visual Studio (and possibly other compilers) does not handle this situation well and can lead to a double-free error. This is because the heap manager keeps track of the allocation status of each block of memory, and when you free a block, it marks it as available for reuse. If you then try to allocate another block at the same location, the heap manager will detect that the block was already freed and raise an exception.

To avoid this issue, you can use std::string constructors that take a const char* pointer instead of a char* pointer, like this:

std::string foo() { const char *path = "/bin/ls"; std::string res(path); return res; }

By using a const char* pointer, you ensure that the original string is not modified and does not need to be freed. Alternatively, you can use std::string constructors that take a char[] array instead of a char* pointer, like this:

char path[] = "/bin/ls"; std::string res(path);

This approach avoids the slicing issue altogether and ensures that the original string is not modified.

[r2ai:0x100003a84]>

trufae commented 2 months ago

thanks for digging in the bug, now it makes sense, but it's a bit fuckedup by c++ standards, it's now fixed in master :) will release 5.9.4 soon

trufae commented 2 months ago

Wait. I’ll dig a bit more because this doesn’t seems to be the correct fix and i’ll see if i can make a small reproducer

Adde2130 commented 2 months ago

Yeah, I noticed today as well, there are some other frees causing heap corruption. Don't know if NOP:ing all of them out is a good idea...

https://github.com/radareorg/r2ghidra/blob/11d3ba7689adef3c255e29def25537db13c4246b/src/core_ghidra.cpp#L308

This line in core ghidra also caused a crash earlier today and it too was caused by heap corruption, specifically after trying to free the char* code in the RCodeMeta struct. Since multiple free calls are causing this issue, I assume the heap corruption must sometime before the getSleighhome call (since the crash occurs there first), and windows doesn't recognize it until a call to free is made.