Bug #317
closed
Added by Peter Manev over 13 years ago.
Updated over 3 years ago.
Description
Please find attached some tests with invalid rule keywords combinations (bad "grammar") that should not be loaded by the engine, nevertheless they are getting loaded.
modifiers and rule keywords - distance,within, depth, offset...
Snort corrected some of the issues they had - ""Improved error checking for invalid combinations of "depth", "offset", "distance", and "within" modifiers in rules. Rules that mix relative and non-relative options on the same content will now cause errors."" - http://blog.snort.org/2010/12/snort-2903-is-coming-soon.html
dated back in Dec 2010, some of them are still not addressed, I believe.
Please find a comparison of invalid rules and if they load or not. I have tested all the bad rules with Sur 1.0.4/1.0.5/git master, Snort 2.8.5.1/2.9.0.5/current beta, the results are in the spreadsheet attached.
Thanks
Files
- Assignee set to Eileen Donlon
@Peter Pan, can you update this to include our 1.1beta3 release and the 2.9.1.2 Snort release?
Updated file attached
Updates include current git master and Snort 2.9.1.2
I've added a sheet to Peter's workbook which lists categories of invalid or bad rules, and color-coded the rules in Peter's list to show the category. The point of listing the categories is that it makes it easier to develop a comprehensive set of rules for testing. Let me know if you have done this already! A lot of these checks have already been implemented but I don't know if they've all been tested.
The "bad" rule checks would be part of the rule analyzer.
I like the idea.
I think it would be good if we can number the categorizations as well i.e.
01 - Semantics Err
02 - Syntax Err
...
We could rewrite the "msg" in the alerts to reflect the categorization.
Then it shouldn’t be difficult to write a script that would test all the rules and reflect the results in one go for every git update/release update.
Other opinions?
- Target version set to TBD
- Assignee changed from Eileen Donlon to Andreas Herz
Quite old :) But tested the rules again against 3.0 and the only rules left are Content-Greater-Than-Dsize.rule and the other Dsiz-*.rule files.
To resolve all the examples we would need to include several checks when parsing dsize rules since the checks depend on several other keywords. Do we want to add this or are we just fine with loading those rules since they shouldn't hurt but also not match?
Ideally - the checks should be there.
Agreed. If a rule can't match due to internal inconsistencies then we should reject it.
As an addition: this rule below -
alert tcp any any -> any any (msg: "la di da"; content: "0a:23:bf hu-ha"; )
loads with 3.1dev (rev d39e575) without err but it shouldn't load since it is missing a sid#.
- Assignee changed from Andreas Herz to Anonymous
- Effort set to medium
- Difficulty set to low
Think it would be nice to refresh this and open tickets for individual issues.
- Assignee set to Community Ticket
- Status changed from New to Assigned
- Assignee changed from Community Ticket to Jason Taylor
suricata-verify tests have been submitted for the invalid rules.
https://github.com/OISF/suricata-verify/pull/57
there is still one rule that is successfully loading in current suricata master branch:
alert udp any any -> any any (msg:"TEST SUCCESFULL - dsize/distance INVALID combination "; dsize:10; content:"boom"; content:"loom"; distance:10; sid:6666663; rev:1;)
#2982 has been created to track the invalid rule loading
- Related to Bug #2982: invalid dsize distance rule being loaded by suricata added
Jason Taylor are you also interested in working on the parsing?
yep, when it's merged we can close it. Thanks!
Ideally I will have some time to work on this in the near future, if it's something time sensitive then by all means someone else can take it. This is all educational so I am stumbling my way through learning :)
That's an ongoing issue for quite some time, so we're happy for every contribution :)
- Status changed from Assigned to Closed
Remaning work is tracked in #2982
Also available in: Atom
PDF