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

Block has wrong return value if it is reused by cascading math beforehand #6175

Open
VisualEhrmanntraut opened this issue Nov 22, 2024 · 15 comments
Labels
Component: Core Issue needs changes to the core Core: MLIL Issue involves Medium Level IL Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround State: Awaiting Triage Issue is waiting for more in-depth triage from a developer Type: Bug Issue is a non-crashing bug with repro steps

Comments

@VisualEhrmanntraut
Copy link
Contributor

Version and Platform (required):

  • Binary Ninja Version: 4.3.6468-dev
  • OS: macOS
  • OS Version: 15.2 Beta (24C5089c)
  • CPU Architecture: M3

Bug Description:
See below:
Screenshot 2024-11-22 at 09 43 10
Screenshot 2024-11-22 at 09 43 25
Result:
Screenshot 2024-11-22 at 09 43 31

Expected Behavior:
Should be return 2

@VisualEhrmanntraut VisualEhrmanntraut changed the title Wrong value returned on eax = CONSTANT, jump to ret instruction Wrong value returned on eax = CONSTANT, jump to ret instruction, inside switch Nov 22, 2024
@xusheng6
Copy link
Member

@VisualEhrmanntraut Any chance you can share the binary? Sorry that I am not connecting the dots

@xusheng6 xusheng6 added the State: Blocked (Customer) Issue is blocked on waiting for a response from a customer label Nov 26, 2024
@VisualEhrmanntraut
Copy link
Contributor Author

@xusheng6 No, sorry, but I can share any further information needed. Just tell me what you need.

@xusheng6
Copy link
Member

@xusheng6 No, sorry, but I can share any further information needed. Just tell me what you need.

Sure, that is totally fine.

Can you show me the LLIL and MLIL as well? I am having some difficulty understanding what is going on

@VisualEhrmanntraut
Copy link
Contributor Author

@xusheng6 Here,
LLIL
Screenshot 2024-11-26 at 14 34 17
Screenshot 2024-11-26 at 14 34 24

MLIL
Screenshot 2024-11-26 at 14 34 53
Screenshot 2024-11-26 at 14 34 59

@xusheng6
Copy link
Member

So it seems to me the switch-case is using r8 as the control variable. However, I see it is compared against 0x2, which indicates some oddly looking code like this:

case 0x2:
    return 0x2;

where does the 0xb0007 come from?

@xusheng6 xusheng6 added State: Awaiting Triage Issue is waiting for more in-depth triage from a developer and removed State: Blocked (Customer) Issue is blocked on waiting for a response from a customer labels Nov 26, 2024
@VisualEhrmanntraut
Copy link
Contributor Author

It's a very weird function
Screenshot 2024-11-26 at 14 39 26

@VisualEhrmanntraut
Copy link
Contributor Author

VisualEhrmanntraut commented Nov 26, 2024

essentially what this function does is convert a version number to an enumerator
i.e. 11.0.5 -> 1, 11.0.7 -> 2, etc

@xusheng6
Copy link
Member

OK I now see how it works. Where is the result variable stored?

@xusheng6 xusheng6 added Type: Bug Issue is a non-crashing bug with repro steps Component: Core Issue needs changes to the core Core: MLIL Issue involves Medium Level IL Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround and removed State: Awaiting Triage Issue is waiting for more in-depth triage from a developer labels Nov 26, 2024
@xusheng6
Copy link
Member

The code uses cascaded subtractions to take values off the incoming r8 register, and return rax values based on that. It seems we somehow screw up the return value tracking

@VisualEhrmanntraut
Copy link
Contributor Author

@xusheng6 eax, afaict
Screenshot 2024-11-26 at 14 57 25
Screenshot 2024-11-26 at 14 57 37

@xusheng6
Copy link
Member

Also @VisualEhrmanntraut , what about other cases after the one which should return 0x2? I see there should be quite a few more of them. Do we deal with it correctly?

@xusheng6 xusheng6 added the State: Awaiting Triage Issue is waiting for more in-depth triage from a developer label Nov 26, 2024
@VisualEhrmanntraut
Copy link
Contributor Author

@xusheng6 All of them seem correct except two, the "2" one and this
Screenshot 2024-11-26 at 14 59 45
that one should be return 0x10

Full function:
Screenshot 2024-11-26 at 15 00 24

@xusheng6
Copy link
Member

I understand what is happening, but do not know why it is happening. Will need someone else to look into it

@VisualEhrmanntraut VisualEhrmanntraut changed the title Wrong value returned on eax = CONSTANT, jump to ret instruction, inside switch Block has wrong return value if it is reused by cascading math beforehand Nov 26, 2024
@SlidyBat
Copy link

I think this may be a duplicate of this issue: #6101

@VisualEhrmanntraut
Copy link
Contributor Author

Hm, maybe. I didn't really know what to look for when creating the issue (or look through 1.5k issues).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Component: Core Issue needs changes to the core Core: MLIL Issue involves Medium Level IL Effort: Medium Issue should take < 1 month Impact: Medium Issue is impactful with a bad, or no, workaround State: Awaiting Triage Issue is waiting for more in-depth triage from a developer Type: Bug Issue is a non-crashing bug with repro steps
Projects
None yet
Development

No branches or pull requests

3 participants