Skip to content

Conversation

@vinieger
Copy link

@vinieger vinieger commented Sep 17, 2025

@Neo23x0
Copy link
Owner

Neo23x0 commented Sep 20, 2025

Hi Vinicius,

I like the rule. I really do.
The only thing that makes me hesitate to merge it is the cost/benefit ratio.
For a vulnerability, that may or may not have been seen being used in the wild and is already patched, I rarely add a rule with regular expression with the complexity of the one your string.
I don't know if we could simplify it without loosing to much coverage.
e.g.

  • remove "nocase"
  • maybe just write it as a hex string with a jump

Like this:

path = ".{1,30}\r"\n
{ 70 61 74 68 20 3D 20 22 [1-30] 0D 22 0A }

I'd like to run a retrohunt on VT to see how many real-world samples we can find.

@vinieger
Copy link
Author

Fair! I'll give it a closer look. Do you do any sort of performance benchmark you can suggest?

@Neo23x0
Copy link
Owner

Neo23x0 commented Sep 23, 2025

It's not just about speed, but the regex also consumes more memory, because the regex state machine has to be held in memory during the evaluations.
YARA is strong in string matching due to the Aho-Corasick algorithm, but regex is evaluated differently and expressions like ".*" can consume a lot of memory depending on the number and kind of matches with a static anchor in the expression.

I only found a single malicious sample on VT and that one looked like a POC:

https://www.virustotal.com/gui/file/a55940339afb68540a172fd72a4c227bf9364bb9b0d6b52ff2f537a8841c2498/content

Screenshot 2025-09-23 at 12 16 49

I would add the rule if it was less complex, but like this the cost/benefit ratio simply isn't good enough.

@vinieger
Copy link
Author

Thanks for sharing the info and screenshot - I don't have VT premium :(
No worries, what you said make sense! I'm happy to take the learnings away.

FYI quickly limited the wildcard to 30 and compared the two with /usr/bin/time -l yr scan:

image

Thanks for the feedback.

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