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

Fix: extract correct mnemonic on IDA Pro #19

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

gluesmith2021
Copy link

Issue

IDA GetDisasm() may not be an unambiguous way to extract an instruction mnemonic.

No-operand Mnemonic

GetDisasm() concatenates the comment with the disassembly. On no-operand mnemonics, the comment is concatenated without any separator, whether it's an automatic or manual comment. For instance:

  • Intel
    • retn ; I'm out!
    • GetDisasm() returns "retn; I'm out!"
  • ARM
    • se_blr # Branch unconditionally
    • GetDisasm() returns "se_blr# Branch unconditionally"

In jvd/ida/ida_utils.py, extracting the mnemonic with GetDisasm(head).split()[0] produce an extra character in those cases, i.e. retn; and se_blr# respectively, which is incorrect.

Intel Prefixes

Intel prefixes such as rep and lock in rep stosd and lock xadd op1, op2 for instance, are also returned by GetDisasm(). jvd thus extract the first word, the prefix, rather than the mnemonic.

Other processors might have such prefix as well.

Proposed Fix

Keep the mnemonic from print_insn_mnem(head) collected three lines above.

In the above example, retn and se_blr are properly returned.

In prefix cases, the fix returns the actual mnemonic only, without the prefix. It seems more semantically correct to return the mnemonic alone than the prefix alone as with original code. It is not clear though whether the prefix should be returned as part of

a) mnemonic
b) operands
c) something else
d) none of the above

The proposed fix goes for d) since it is not returned at all.

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

Successfully merging this pull request may close these issues.

1 participant