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

Variable name propagation #2558

Closed
yrp604 opened this issue Jul 23, 2021 · 3 comments
Closed

Variable name propagation #2558

yrp604 opened this issue Jul 23, 2021 · 3 comments
Assignees
Labels
Component: UI Issue needs changes to the user interface Effort: High Issue should take > 1 month Impact: High Issue adds or blocks important functionality Type: Enhancement Issue is a small enhancement to existing functionality
Milestone

Comments

@yrp604
Copy link
Contributor

yrp604 commented Jul 23, 2021

Is your feature request related to a problem? Please describe.
Reversing code is hard, and I need all the help I can get.

Describe the solution you'd like
Some APIs provide named parameters in their type signatures. For example, opening and reading a file currently might look something like this:

HANDLE rax_1 = CreateFileA(...)
BOOL rax_2 = ReadFileA(rax_1, ...)

If the type signatures provide file names, this could potentially default to something like this:

HANDLE hFile_1 = CreateFileA(...)
BOOL  rax_2 = ReadFileA(hFile_1, ...)

Notably in this case, rax_1 now autodefault to being named hFile_1 due to it being consumed by ReadFileA. This behavior (as in the name coming from the use site) isn't the specific request, but rather just an example. This request is general -- through some mechanism it would be great if variable names could be inferred, even in a subset of cases.

Describe alternatives you've considered
The alternative here is not inferring names, which would be kinda a bummer so I didn't consider it.

Additional context
I believe this is a recent IDA feature.

@yrp604 yrp604 added the Type: Enhancement Issue is a small enhancement to existing functionality label Jul 23, 2021
@Ayrx
Copy link

Ayrx commented Jul 23, 2021

An API for plugins to add additional logic for inferring the names would be nice as well. You can technically already do so today by walking the IL tree but a callback style API would be nicer I think.

@plafosse plafosse changed the title Automatically name variables off function signatures when available Variable name propagation Feb 9, 2022
@plafosse plafosse added Effort: High Issue should take > 1 month Impact: High Issue adds or blocks important functionality labels Feb 9, 2022
@fuzyll fuzyll added the Component: UI Issue needs changes to the user interface label May 17, 2023
@plafosse plafosse added this to the Coruscant milestone May 24, 2023
@D0ntPanic
Copy link
Member

Added in 3.5.4358

@yrp604
Copy link
Contributor Author

yrp604 commented Jun 27, 2023

I think there are still problems with this. For example, here is notepad.exe:

14000cccc  int64_t DetermineFileTypeEncoding()

14000cce5      void var_468
14000cce5      int64_t rax_1 = __security_cookie ^ &var_468
14000ccf9      g_defaultEncoding // <- what is this?
14000cd0c      int32_t var_448 = 3
14000cd18      PWSTR lpFileName // <- this is presumably stack allocated?
14000cd18      HANDLE rax_2 = CreateFileW(lpFileName, dwDesiredAccess: 0x80000000, dwShareMode: FILE_SHARE_READ, lpSecurityAttributes: nullptr, dwCreationDisposition: OPEN_EXISTING, dwFlagsAndAttributes: FILE_ATTRIBUTE_NORMAL, hTemplateFile: nullptr) // <- right now it looks like were calling CreateFile on uninit stack contents?

This successfully created a variable named lpFileName, however this should have been an argument to DetermineFileTypeEncoding:

14000cccc  int64_t DetermineFileTypeEncoding()

14000cccc  48895c2410         mov     qword [rsp+0x10 {__saved_rbx}], rbx
14000ccd1  4889742418         mov     qword [rsp+0x18 {__saved_rsi}], rsi
14000ccd6  57                 push    rdi {__saved_rdi}
14000ccd7  4881ec60040000     sub     rsp, 0x460
14000ccde  488b050b770200     mov     rax, qword [rel __security_cookie]
14000cce5  4833c4             xor     rax, rsp {var_468}
14000cce8  4889842450040000   mov     qword [rsp+0x450 {var_18}], rax
14000ccf0  488364243000       and     qword [rsp+0x30 {var_438}], 0x0
14000ccf6  4533c9             xor     r9d, r9d  {0x0}
14000ccf9  8b1db1880200       mov     ebx, dword [rel g_defaultEncoding]
14000ccff  ba00000080         mov     edx, 0x80000000
14000cd04  c744242880000000   mov     dword [rsp+0x28 {var_440}], 0x80
14000cd0c  c744242003000000   mov     dword [rsp+0x20 {var_448}], 0x3
14000cd14  458d4101           lea     r8d, [r9+0x1]
14000cd18  48ff15c1d00100     call    qword [rel CreateFileW] ; <-- rcx is passed here

Looking at the asm, we can we see rcx isn't touched before the call to CreateFileW.

notepad.exe (2).txt

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: UI Issue needs changes to the user interface Effort: High Issue should take > 1 month Impact: High Issue adds or blocks important functionality Type: Enhancement Issue is a small enhancement to existing functionality
Projects
None yet
Development

No branches or pull requests

5 participants