Project

General

Profile

Actions

Bug #5799

closed

detect: sigs using DETECT_SM_LIST_PMATCH can break other signatures

Added by Andreas Herz almost 2 years ago. Updated over 1 year ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:
Needs backport to 6.0

Description

With the following smb pcap and two simple rules, we were able to reproduce a severe bug in 6.0.9 and 7.0 Beta/Master.

The following two rules trigger the bug:
```
alert ssh $HOME_NET any -> any any (msg:"pcre without content"; pcre:"/rabbit/B"; sid:1; rev:1;)
alert smb $HOME_NET any -> any any (msg:"smb share content"; smb.share; content:"C"; sid:2; rev:1;)
```
The second rule for smb would match and trigger 4 alerts on itself, but as soon as the first rule is parsed and registered as well, the second rule won't match anymore.

If you add a `content` match to the first rule it works again and generates 4 alerts:
```
alert ssh $HOME_NET any -> any any (msg:"pcre with content"; content:"r"; pcre:"/rabbit/B"; sid:1; rev:1;)
alert smb $HOME_NET any -> any any (msg:"smb share content"; smb.share; content:"C"; sid:2; rev:1;)
```

It seems to be related to `DETECT_SM_LIST_PMATCH` somehow overwriting parts that are relevant for the second rule.

The impact could be quite relevant, once we added such a dummy rule we did some tests and for example within a malicious pcap used against the ET ruleset, the alerts went down from around 890 to 850 so a rule that doesn't even match can have an impact on the amount of valid alerts.

Using the `pcre` is just on example, other keywords that use `DETECT_SM_LIST_PMATCH` also have the potential to break other rules.


Files

smb.pcap (735 KB) smb.pcap Andreas Herz, 01/19/2023 08:54 AM
0001-detect-remove-flush.patch (794 Bytes) 0001-detect-remove-flush.patch Eric Leblond, 01/19/2023 10:09 PM

Related issues 1 (0 open1 closed)

Related to Suricata - Bug #4759: TCP DNS query not found when tls filter is activeClosedJason IshActions
Actions #1

Updated by Andreas Herz almost 2 years ago

Actions #2

Updated by Andreas Herz almost 2 years ago

There is a more verbose pcap from https://www.malware-traffic-analysis.net/2019/07/05/2019-07-05-Ursnif-with-Trickbot-and-IcedID.pcap.zip which is public and I was able to create another scenario where a valid signature is not matching anymore:

alert ssh $HOME_NET any -> any any (msg:"pcre without content and no match"; pcre:"/rabbit/"; sid:1; rev:1;)
alert krb5 $HOME_NET any -> any any (msg:"krb match"; krb5_cname; content:"marlo"; sid:3; rev:1;)

If both rules are included, 0 alerts, once the first rule is commented the second rule triggers 6 alerts. So it's not only related to smb but in that case also has an impact on kerberos.
The interesting case on this one is, that adding a content match to the first rule doesn't help.

Actions #3

Updated by Eric Leblond almost 2 years ago

I fear I've missed something but the attached patch is fixing the issue.

In the C code, we just set it:

grep -r STREAM_FLUSH src/
src/detect.c:        flow_flags &=~ STREAM_FLUSH;
src/detect.c:    TRACE_SID_TXS(s->id, tx, "FLUSH? %s", (flow_flags & STREAM_FLUSH)?"true":"false");

In rust, it is just defined:

grep -r STREAM_FLUSH rust/src/
rust/src/core.rs:pub const STREAM_FLUSH:    u8 = 0x80;

So this is unused in the code. Maybe we have a conflict there. But at least it should not be set there because it is not used anywhere.

Actions #5

Updated by Jason Ish over 1 year ago

  • Related to Bug #4759: TCP DNS query not found when tls filter is active added
Actions #6

Updated by Jason Ish over 1 year ago

I assume this smb.pcap is public?

Actions #7

Updated by Andreas Herz over 1 year ago

Jason Ish wrote in #note-6:

I assume this smb.pcap is public?

yep I extracted this flow from the MTA public pcap

Actions #8

Updated by Jason Ish over 1 year ago

  • Status changed from New to Closed
Actions #9

Updated by Victor Julien over 1 year ago

  • Subject changed from Signatures using DETECT_SM_LIST_PMATCH could break other signatures on detection to detect: sigs using DETECT_SM_LIST_PMATCH can break other signatures
  • Priority changed from High to Normal
  • Label Needs backport to 6.0 added
Actions #10

Updated by Victor Julien over 1 year ago

  • Private changed from Yes to No
Actions

Also available in: Atom PDF