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.

Sign up for GitHub

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

Jump to bottom

Check for impossible conditions when using uint() operators. #1918

Open
tlansec opened this issue May 5, 2023 · 5 comments · May be fixed by #1919
Open

Check for impossible conditions when using uint() operators. #1918

tlansec opened this issue May 5, 2023 · 5 comments · May be fixed by #1919
Labels
feature-request

Comments

@tlansec
Copy link
Contributor

tlansec commented May 5, 2023

Is your feature request related to a problem? Please describe.
Sometimes when writing rules I'll use the wrong value of x for uintx type rules meaning the condition will never match. For example

condition:
    uint32(0) == 0xABCD

Describe the solution you'd like
Troubleshooting a condition like this is relatively easy once you figure out what is going on but it can be easy to gloss over. It would be useful if when compiling rules the compiler checked that the right handside contained a number of bytes that could plausibly match the left hand side,

Describe alternatives you've considered
I could just write better rules I guess :(
@wxsBSD
Copy link
Collaborator

wxsBSD commented May 5, 2023

I'm not sure I like this. So just because I want to check that a 32bit value is zero I have to specify it as 0x00000000? Seems excessive and not how other languages work.

@plusvic
Copy link
Member

plusvic commented May 6, 2023

I think @wxsBSD is right.

Why uint32(0) == 0xABCD should be incorrect? This is not really an impossible condition. 0xABCD is the same as 0x0000ABCD, and most people will expect both to be equivalent.

uint8(0) == 0xABCDE is a different matter. This case is actually imposible, and a warning could be useful here.

@wxsBSD
Copy link
Collaborator

wxsBSD commented May 6, 2023

I can see the warning you describe here as being useful. I'll add that soon.

@plusvic
Copy link
Member

plusvic commented May 6, 2023

Related: VirusTotal/yara-x#23

@tlansec
Copy link
Contributor Author

tlansec commented May 6, 2023

I hadn't realised that uint32(x) = 0x00 checked all four bytes were null (it makes sense now that you say it , I just hadn't realised it).

wxsBSD added a commit to wxsBSD/yara that referenced this issue May 7, 2023
@wxsBSD
Warn on always false integer reads and comparisons.
0839ab3 
When reading an integer of a specific size and comparing it to an integer of a
larger size where any of the upper bits are set we will now emit a warning
because the comparison is always false.

These will always evaluate to false because the "extra" bytes are non-zero:

uint8(0) == 0x1100
uint16(0) == 0x110000
uint32(0) == 0x1100000000

While I'm here, move a test into a better place for it. I added it in the wrong
place in ccbc405.

Fixes VirusTotal#1918.
@wxsBSD wxsBSD linked a pull request May 7, 2023 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
feature-request
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Warn on always false integer reads and comparisons. wxsBSD/yara
3 participants
@plusvic @wxsBSD @tlansec