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

Constant propogation wrongly calcuates the value after certain rol instructions #4252

Closed
xusheng6 opened this issue Apr 20, 2023 · 3 comments
Assignees
Labels
Component: Core Issue needs changes to the core Effort: Trivial Issue should take < 1 day Impact: Low Issue is a papercut or has a good, supported workaround Type: Bug Issue is a non-crashing bug with repro steps
Milestone

Comments

@xusheng6
Copy link
Member

xusheng6 commented Apr 20, 2023

In the following code,

sub_0:
00000000  mov     eax, 0x2e56d4fc
00000005  rol     eax, 0xff  {sub_0}

the value of eax should be 0x172b6a7e after the rol instruction. However, the constant propagation thinks its value is 0x0 (see the sub_0 annotation).

Another example:

00007ff74301b4d8  xor     edx, 0xa77f0ed8  {0x8406fb4b}
00007ff74301b4de  rol     edx, 0x2c  {0x0}

0x2c is 0b101100. When we try to calculate its value and shift right, since BN is 64 bit, we will shift the input value, 0x8406fb4b, 0x2c times. However, in the real CPU it will only shift 0b1100 (12) times. This causes the result to be different.

This is actually quite subtle. We only get the value wrong when the target is 32 bit, and the shift count is different when truncated to its lowest 5 or 6 bits

@xusheng6 xusheng6 assigned xusheng6 and unassigned bpotchik May 11, 2023
@xusheng6 xusheng6 added Impact: Low Issue is a papercut or has a good, supported workaround Effort: Low Issue should take < 1 week Type: Bug Issue is a non-crashing bug with repro steps Component: Core Issue needs changes to the core labels May 11, 2023
@xusheng6
Copy link
Member Author

We are not calculating the values involving shift-rotate when the shift is larger than the width of the integer. This should be a relatively easy fix.

@xusheng6 xusheng6 added Effort: Trivial Issue should take < 1 day and removed Effort: Low Issue should take < 1 week labels May 11, 2023
@xusheng6
Copy link
Member Author

xusheng6 commented Jul 5, 2023

The binary triggering this bug is: https://crackmes.one/crackme/643ac6f133c5d439389128c9

@xusheng6
Copy link
Member Author

xusheng6 commented Aug 4, 2023

This also affects the shl instruction. For example, in the following code, we think the value of eax is 0x0 after the code:

1400025e0  mov     eax, 0x1
1400025e5  shl     eax, 0x21  {0x0}

Though its actual value will be 0x2. The reason is that 0x21 == 0b100001. And since the operand is 32 bits, the 0x21 will be truncated to its lowest 5 bits, leaving it as 0x1. And shift eax left 1 bit gives us a value of 0x2.

However, when BN tries to calculate the value, it does not account for the truncation. And since BN is a 64 bit application, the shl instruction uses the lowest 6 bits. So it will just shift eax 0x21 times, making the lowest 32 bits of the value zero. Thus it thinks the final value of eax is 0x0

@xusheng6 xusheng6 closed this as completed Aug 8, 2023
@xusheng6 xusheng6 added this to the Coruscant milestone Aug 8, 2023
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 Effort: Trivial Issue should take < 1 day Impact: Low Issue is a papercut or has a good, supported workaround Type: Bug Issue is a non-crashing bug with repro steps
Projects
None yet
Development

No branches or pull requests

2 participants