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 some compiler edge cases. #1709

Merged
merged 3 commits into from
May 17, 2022

Conversation

wxsBSD
Copy link
Collaborator

@wxsBSD wxsBSD commented May 16, 2022

Add a warning when "0 of them" is used. Point to some new documentation that
explains why "none of them" is preferred.

Handle some edge cases, which are all now errors:

  • -1 of them
  • "foo" of them
  • /foo/ of them

And this one is also an error if the external variable "x" is a negative integer
or a string:

  • x of them

I've also made sure this works with a construct like "0 + x of them" where x is
defined as a negative integer.

Add test cases for as many of these as I can. I don't have test cases for the
external variable case, but it does work as expected. Here is the output of a
rule using external variables where the variable is defined to be 1, 0, -1 and
foo:

wxs@mbp yara % cat rules/test.yara
rule a {
  strings:
    $a = "FreeBSD"
  condition:
    x of them
}
wxs@mbp yara % ./yara -d x=1 rules/test.yara /bin/ls
a /bin/ls
wxs@mbp yara % ./yara -d x=0 rules/test.yara /bin/ls
warning: rule "a" in rules/test.yara(5): Consider using "none" keyword, it is less ambiguous. Please see https://yara.readthedocs.io/en/stable/writingrules.html#sets-of-strings-1 for an explanation.
wxs@mbp yara % ./yara -d x=-1 rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "-1"
wxs@mbp yara % ./yara -d x=foo rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "string in for_expression is invalid"
wxs@mbp yara %

Add a warning when "0 of them" is used. Point to some new documentation that
explains why "none of them" is preferred.

Handle some edge cases, which are all now errors:

* -1 of them
* "foo" of them
* /foo/ of them

And this one is also an error if the external variable "x" is a negative integer
or a string:

* x of them

I've also made sure this works with a construct like "0 + x of them" where x is
defined as a negative integer.

Add test cases for as many of these as I can. I don't have test cases for the
external variable case, but it does work as expected. Here is the output of a
rule using external variables where the variable is defined to be 1, 0, -1 and
foo:

```
wxs@mbp yara % cat rules/test.yara
rule a {
  strings:
    $a = "FreeBSD"
  condition:
    x of them
}
wxs@mbp yara % ./yara -d x=1 rules/test.yara /bin/ls
a /bin/ls
wxs@mbp yara % ./yara -d x=0 rules/test.yara /bin/ls
warning: rule "a" in rules/test.yara(5): Consider using "none" keyword, it is less ambiguous. Please see https://yara.readthedocs.io/en/stable/writingrules.html#sets-of-strings-1 for an explanation.
wxs@mbp yara % ./yara -d x=-1 rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "-1"
wxs@mbp yara % ./yara -d x=foo rules/test.yara /bin/ls
error: rule "a" in rules/test.yara(5): invalid value in condition: "string in for_expression is invalid"
wxs@mbp yara %
```
libyara/grammar.y Outdated Show resolved Hide resolved
If you specify "2 of ($a)" it is now a warning because it will always evaluate
to false, and is quite possibly not what you wanted to write. This is true for
string sets, rule sets and string sets in a range like "2 of ($a) in (0..10)".

If you are using an integer range it is now an error if the lower bound is
greater than the upper bound.

To support rule sets I needed to modify yr_parser_emit_pushes_for_rules() to
use a count parameter, which it will update with the number of pushed rules.
This is identical to how yr_parser_emit_pushes_for_strings() works.

While I'm here, remove the link from the warning message for the "none" keyword.
@wxsBSD
Copy link
Collaborator Author

wxsBSD commented May 17, 2022

I fixed the message and also added a bunch more warnings and one more error. Specifically these are now warnings:

2 of ($a) is a warning because it will always evaluate to false and is likely not what the user meant to happen. The same is true for rule sets and strings in a range like 2 of ($a) in (0..10).

In ranges, if we can statically determine the bounds it is now an error if the lower bound is greater than the upper bound.

@plusvic plusvic merged commit a5903e1 into VirusTotal:master May 17, 2022
@wxsBSD wxsBSD deleted the 0_warning_and_negative_fix branch May 17, 2022 16:24
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.

2 participants