Add /dev/ rule #148
Merged
Add /dev/ rule #148
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
egibs
force-pushed
the
add-dev-detection
branch
from
c2bde4d to
4ec9d46
Compare
|
Thanks! YARA rules are both fun and frustrating to write :) I see that this PR is trying to do the right thing within YARA limitations. It's good, but I wanted to share a case where I think we can improve it. If a file contains /dev/null and /dev/stdin, this rule returns nothing: I don't know the best way to handle this case in yara: where you want to exclude a single result, but still allow other matches within a file. Looking at VirusTotal/yara#1452 I'm not sure there is a native way to do this correctly in YARA. My two suggestions:
I'm kind of siding toward the first option, but I think the second is an eventuality. Here's an example of a simplified version of your rule that I think delivers what you might be trying to achieve: What do you think? |
|
One last tip: for performance reasons, try to avoid + in regexps when possible, and use alternatives that contain limits, for example |
I agree with option one for now; though, I do think that option two will make it easier to exclude more nuanced paths in the future. I'll work on implementing your suggestions! |
added 2 commits
April 20, 2024 11:14
|
@tstromberg -- addressed your comment(s) in I used Here's the output prior to me updating the I tried to reproduce this output inside of a container and Maybe some unintended behavior to investigate further? |
tstromberg
reviewed
Apr 23, 2024
rules/ref/path/dev.yara
Outdated
| description = "path reference within /dev" | ||
| strings: | ||
| $path = /\/dev\/[a-z\.\-\/]{1,16}/ | ||
| $ignore_null = /\/dev\/nu[l]{1,2}/ |
There was a problem hiding this comment.
I don't think it's necessary outside of what I noted in this comment.
I'll tweak the rule and check the output again.
There was a problem hiding this comment.
Yea, reverting to /dev/null results in this:
MED ref/path/dev path reference within /dev /dev/nul
/dev/stderr
/dev/stdout
There was a problem hiding this comment.
Also saw this when tweaking the rule:
MED ref/path/dev path reference within /dev /dev/null.
There was a problem hiding this comment.
Reverted to /dev/null in 011f205 (#148).
After a bit of experimentation, the original rule was matching /dev/nul1 which is still odd.
I spent a couple of hours last night comparing the output of docker run -d debian; docker container export <ID> and go run . --oci debian and the former never showed any rule matches like that.
It could be due to how crane exports the layers of the image resulting in artifacts like this but that's pure conjecture on my part. I did make sure that the actual Bincapz code wasn't modifying the match strings, though.
|
@tstromberg -- I fixed the tests, by the way; |
vaikas
approved these changes
Apr 23, 2024
|
Yay! Thanks for your PR and for sticking through all the rough edges. |
egibs
pushed a commit
to egibs/malcontent
that referenced
this pull request
Aug 5, 2024
* Add /dev/ rule * Address PR comments * Remove extra () * Revert to /dev/null; update path * Fix tests
egibs
pushed a commit
to egibs/malcontent
that referenced
this pull request
Sep 25, 2024
* Add /dev/ rule * Address PR comments * Remove extra () * Revert to /dev/null; update path * Fix tests
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes: #147
I wanted to try my hand at writing some YARA rules -- this PR adds a detection for
/dev/paths while excluding/dev/nulland/dev/shm/...since there are already detections for these two paths.I tested this with
$ go run . --oci python-- for example:There were a total of 139 matches for this rule in
python.