Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bad fix of abspath issue #13301

Closed
a1ext opened this issue Mar 6, 2019 · 0 comments
Closed

Bad fix of abspath issue #13301

a1ext opened this issue Mar 6, 2019 · 0 comments

Comments

@a1ext
Copy link
Contributor

a1ext commented Mar 6, 2019

Work environment

Questions Answers
OS/arch/bits (mandatory) Windows 7 x64
File format of the file you reverse (mandatory) PE
Architecture/bits of the file (mandatory) x86/64
r2 -v full output, not truncated (mandatory) radare2 3.4.0-git 21113 @ windows-x86-64 git.3.3.0-91-gd63d4f228 commit: d63d4f2 build: 07.03.2019__ 0:07:04,93

Expected behavior

Radare2 don't crash, copying of files works well even if they have unicode name or unicode characters in a full path to it

Actual behavior

Radare2 crashes because of heap corruption.

Steps to reproduce the behavior

Why

I think it is related to the following PR, I have some notices why:

  • If radare2 compiled with UNICODE support, WinAPI function names will be substituted as wide- version of them, like GetFullPathName -> GetFullPathNameW, CopyFile -> CopyFileW, etc.
    at the following line
    image
    we pass utf8 string to the func, which expects wide string, and we say it will be MAX_PATH long, which means MAX_PATH * sizeof(wchar_t) which is wrong, heap gets corrupted. Note To support Unicode we have to use Wide-only WinAPI funcs, don't force to use ANSI ones!

  • r_str_argv_free has to be used to free files, cmd is not being freed -> here
    image

  • CopyFileW here
    image

expects wide-string there, so s and d have to be converted from utf8 to utf16 with help of appropriate function like r_sys_conv_utf8_to_utf16/r_sys_conv_utf8_to_utf16_l

@GustavoLCR can you take a look?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

1 participant