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 issue #999 #1001

Merged
merged 2 commits into from Dec 13, 2018
Merged

Fix issue #999 #1001

merged 2 commits into from Dec 13, 2018

Conversation

plusvic
Copy link
Member

@plusvic plusvic commented Dec 13, 2018

No description provided.

@bnbdr
Copy link

bnbdr commented Dec 17, 2018

I would make sure all other opcodes don't suffer from similar issues.

@plusvic
Copy link
Member Author

plusvic commented Dec 19, 2018

OP_LENGTH (and OP_OFFSET with a bit more difficulty) could be used to read the canary provided that you can write a YR_MATCH structure somewhere and get its address in order to push it into the virtual stack. Writing the YR_MATCH structure in the virtual stack is possible, but I didn't find a way of knowing its address.

@bnbdr
Copy link

bnbdr commented Dec 20, 2018

...I didn't find a way of knowing its address.

Faking YR_MATCH structure is possible because the bytecode can still infer the location of the vstack as I showed with the PoC and the function object.

Regardless, I did find the following:

  • It's possible to read the canary value using OP_OFFSET without finding the vstack and faking a YR_MATCH struct on it. I can explain further if needed.

  • OP_MATCH_RULE could theoretically be used to set the LSB of any address.

  • The canary value is limited to a 15-bit unsigned value (at least on windows):

// stdlib.h
#define RAND_MAX 0x7fff

@plusvic
Copy link
Member Author

plusvic commented Dec 20, 2018

It's possible to read the canary value using OP_OFFSET without finding the vstack and faking a YR_MATCH struct on it. I can explain further if needed.

I'm curious about this.

@bnbdr
Copy link

bnbdr commented Dec 20, 2018

Basically pushing a shifted pointer to a real YR_OBJECT before issuing OP_OFFSET can cause the VM to read that object's parent's identifier and canary and put the addition of those on the virtual stack.

Then it's a matter of doing some calculations to reach the original canary value.

I've written a rule (yarasm, compiled) that compares the canary using both methods (OP_OFFSET and OP_COUNT).

It should exit cleanly if they match (i.e. no assertions).

@bnbdr
Copy link

bnbdr commented Jan 23, 2019

@plusvic I can elaborate if necessary.

@plusvic
Copy link
Member Author

plusvic commented Jan 23, 2019

Any additional info would be helpful. I haven't enough time to look into this in detail. However I'm wondering, given the design of the VM, if is going to be actually possible to close all the holes.

@bnbdr
Copy link

bnbdr commented Jan 23, 2019

Easiest "patch" would be to break compatibility by requiring a command-line switch to allow running compiled rules together with classifying compiled rules as "potentially dangerous".

This prevents uninformed users from running possibly malicious rules without their intention and shifts the responsibility.

additional info would be helpful

I'll see what I can conjure up

@plusvic
Copy link
Member Author

plusvic commented Jan 23, 2019 via email

@plusvic plusvic deleted the issue_999 branch April 29, 2019 10:11
tarterp pushed a commit to mandiant/yara that referenced this pull request Mar 31, 2022
* Add additional check in OP_COUNT for making sure that the string pointer is not a fake one.

* Initialize scratch memory in order to avoid maliciously crafted YARA rules from reading values left in the stack.
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.

None yet

2 participants