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

Incorrect loop detection for a section with an non-conditional jump inside it #7

Open
justanotheranonymoususer opened this issue Dec 15, 2016 · 6 comments
Milestone

Comments

@justanotheranonymoususer
Copy link
Contributor

An example:
screenshot

@ThunderCls ThunderCls added the bug label Dec 15, 2016
@ThunderCls
Copy link
Owner

Actually I got second thoughts on this one. What is the "loop" definition?

@justanotheranonymoususer
Copy link
Contributor Author

Let me try...

A loop in an assembly listing is a block of code which runs repeatedly multiple times, until a condition is met.

Now, this is a loop in terms of the definition:

@label:
    dec ecx
    test ecx, ecx
    jnz @label

This isn't:

@label:
  > dec ecx
    jmp @far_away_1
  > inc ecx
    jmp @far_away_2
  > xor ecx, ecx
    jmp @label

(> means that an x-ref exists)

@ThunderCls
Copy link
Owner

Ok, then by the definition of loop...would this be incorrect?

@label:
  > cmp dword [eax + 5], ecx
    jne @label2
  > inc edx
    jmp @far_away_2
@label2:
  > inc ecx
    jmp @label

@justanotheranonymoususer
Copy link
Contributor Author

By the definition, it isn't a loop. The commands 1, 2, 5, 6 are a loop, but it's not a contiguous block. Whether the whole block should be marked is a tricky question. What if the number of commands which are not part of the loop is very large? The mark would mis-inform. Perhaps a good solution would be to have the line next to the commands which are not actually part of the loop grayed out. Like this:

image

@ThunderCls
Copy link
Owner

Ok, I see your point @justanotheranonymoususer, unfortunately x64dbg doesnt´t allow that kind of grayed out lines, besides that, making a more complex loops detection algo is not something I got much interest on making right now, anyways PRs are open if you would like to contribute in this matter or any other feature or improvement, anything is well received. Thanks

@justanotheranonymoususer
Copy link
Contributor Author

Well, this is the ideal option, and I understand that it's not trivial. The original request was just to not mark code which doesn't contain loops at all. Like this one:

@label:
  > dec ecx
    jmp @far_away_1
  > inc ecx
    jmp @far_away_2
  > xor ecx, ecx
    jmp @label

In any case, thank you for the plugin, it already is awesome and very helpful!

@ThunderCls ThunderCls added this to the v3.0 milestone May 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants