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 compiler crash. #1992

Closed
wants to merge 1 commit into from
Closed

Conversation

wxsBSD
Copy link
Collaborator

@wxsBSD wxsBSD commented Oct 23, 2023

The compiler hits some asserts when compiling this rule:

rule r{condition:for 5f in(""):(f

It looks like the value of the expression here is undefined, so when trying to use the buffer_id and offset fields of the sized_string_ref we end up using the upper and lower halves of YR_UNDEFINED as if they are real values.

I'm not sure I like checking for YR_UNDEFINED using the integer field of the union, so I'm open to better ideas here.

Found by: Google clusterfuzz

The compiler hits some asserts when compiling this rule:

```
rule r{condition:for 5f in(""):(f
```

It looks like the value of the expression here is undefined, so when trying to
use the buffer_id and offset fields of the sized_string_ref we end up using the
upper and lower halves of YR_UNDEFINED as if they are real values.

I'm not sure I like checking for YR_UNDEFINED using the integer field of the
union, so I'm open to better ideas here.

Found by: Google clusterfuzz
@@ -1318,6 +1318,10 @@ regexp
boolean_expression
: expression
{
// If the value of an expression is ever undefined we have a problem.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would say that an expression can be undefined, in fact, most expressions will be undefined because their value can't be known at compile time. I'm not understanding the reasoning behind this fix.

Copy link
Collaborator Author

@wxsBSD wxsBSD Nov 15, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It fixes a compiler crash, which I have used in the test case. I don't think the fix is technically correct but I did want to bring it up as a fuzzer found it and it got auto assigned to me. ;)

@plusvic plusvic closed this in b26b000 Nov 16, 2023
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