Skip to content
This repository was archived by the owner on May 16, 2025. It is now read-only.

Conversation

wxsBSD
Copy link
Contributor

@wxsBSD wxsBSD commented Dec 23, 2022

This adds support for text string sets (VirusTotal/yara#1787).

This adds support for text string sets (VirusTotal/yara#1787).
@wxsBSD
Copy link
Contributor Author

wxsBSD commented Dec 23, 2022

I'm not sure we can add test cases for what the official compiler considers syntax errors:

for any s in ("a", 0): (s == 0) is an error because you can't mix types, but we don't catch it and I'm not sure we can without a fair bit of work.

for any s in ("a"): (s == 0) is an error because of the comparison between a string and an integer, which we currently don't catch.

I think to fix these would require a lot of work in the parser that probably isn't worth it?

@plusvic
Copy link
Member

plusvic commented Dec 29, 2022

I'm not sure we can add test cases for what the official compiler considers syntax errors:

for any s in ("a", 0): (s == 0) is an error because you can't mix types, but we don't catch it and I'm not sure we can without a fair bit of work.

for any s in ("a"): (s == 0) is an error because of the comparison between a string and an integer, which we currently don't catch.

I think to fix these would require a lot of work in the parser that probably isn't worth it?

I agree, gyp is only a parser, its mission is creating the AST for YARA rules, and such semantics checks are sometimes impossible. For example, the following two conditions are invalid for the same reason:

for any s in ("a"): (s == 0) 
for any s in ("a"): (s == x)  // where X is some external variable of type integer. 

With a parser like gyp you could be able to detect that the first condition is invalid, but the second one is impossible to detect unless you go beyond parsing, but that's actually outside the of the core mission for a parser.

@plusvic plusvic merged commit 9343e33 into VirusTotal:master Dec 29, 2022
@wxsBSD wxsBSD deleted the text_string_sets branch December 29, 2022 13:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants