Project

General

Profile

Actions

Bug #3109

open

dcerpc engine not generating alerts

Added by Travis Green about 2 years ago. Updated 7 months ago.

Status:
Feedback
Priority:
Normal
Target version:
Affected Versions:
Effort:
Difficulty:
Label:

Description

Rules using dce* keywords do not generate an alert despite matching packet contents. For example, given these two rules:

alert tcp any any -> $HOME_NET any (msg:"TGI LATERAL DCERPC ATSVC v1.0 Bind UUID 1ff70682-0a51-30e8-076d-740be8cee98b"; flow:established; dce_iface:1ff70682-0a51-30e8-076d-740be8cee98b,any_frag; reference:url,401trg.com/an-introduction-to-smb-for-network-security-analysts/; classtype:attempted-admin; sid:2610115; rev:1; metadata:notworking;)

alert tcp any any -> $HOME_NET any (msg:"TGI LATERAL DCERPC ATSVC v1.0 Bind UUID"; flow:established; content:"|82 06 f7 1f 51 0a e8 30 07 6d 74 0b e8 ce e9 8b|"; reference:url,401trg.com/an-introduction-to-smb-for-network-security-analysts/; classtype:attempted-admin; sid:2610113; rev:1;)

and this packet: https://imgur.com/a/necQQvy

An alert is only generated for the rule that does not use dce_iface. Pcap attached for repro.


Files

20171220_smb_at_schedule.pcap (3.53 KB) 20171220_smb_at_schedule.pcap Travis Green, 08/09/2019 11:01 PM
Actions #1

Updated by Travis Green about 2 years ago

Eric Leblond had this deeper analysis to offer:

regit 4:39 PM
ok so here we are:
Diff
diff --git a/rust/src/smb/detect.rs b/rust/src/smb/detect.rs
index c3b890eef..1ab8f3f4d 100644
--- a/rust/src/smb/detect.rs
+++ b/rust/src/smb/detect.rs
@@ -182,13 +182,13 @@ pub extern "C" fn rs_smb_tx_get_dce_iface(state: &mut SMBState,
                                             ver_check: u16)
                                             -> u8
 {
-    let is_dcerpc_request = match tx.type_data {
+/*    let is_dcerpc_request = match tx.type_data {
         Some(SMBTransactionTypeData::DCERPC(ref x)) => { x.req_cmd == 1 },
         _ => { false },
     };
     if !is_dcerpc_request {
         return 0;
-    }
+    } */
     let ifaces = match state.dcerpc_ifaces {
         Some(ref x) => x,
         _ => {
@@ -197,12 +197,13 @@ pub extern "C" fn rs_smb_tx_get_dce_iface(state: &mut SMBState,
     };

     let uuid = unsafe{std::slice::from_raw_parts(uuid_ptr, uuid_len as usize)};
-    SCLogDebug!("looking for UUID {:?}", uuid);
+    SCLogNotice!("looking for UUID {:?}", uuid);

     for i in ifaces {
-        SCLogDebug!("stored UUID {:?} acked {} ack_result {}", i, i.acked, i.ack_result);
+        SCLogNotice!("stored UUID {:?} acked {} ack_result {}", i, i.acked, i.ack_result);

-        if i.acked && i.ack_result == 0 && i.uuid == uuid {
+        //if i.acked && i.ack_result == 0 && i.uuid == uuid {
+        if i.uuid == uuid {
             if match_version(ver_op as u8, ver_check as u16, i.ver) {
                 return 1;
             }

First code is working only on request
second if is forcing check on fact the iface is acked and the ack_result is 0
First point: this contredict a previous commit https://github.com/OISF/suricata/commit/c4b56ca28917eb460ea9eb223b9bc98fbb9ee1d8
so we have a bug
second point: I don't know enough the protocol but for me, if we specify a match on iface and we have iface we should have a match any way
@tgreen with this patch I got 2 alerts
on signature that was failing
tgreen profile image    
regit 4:55 PM
4.1.x with rust and without is not matching too

Actions #2

Updated by Eric Leblond about 2 years ago

Side note: the pcap needs stream.midstream to true to get properly analysed but this does not alert correctly after that.

Actions #3

Updated by Andreas Herz about 2 years ago

  • Assignee set to OISF Dev
  • Target version set to TBD
Actions #4

Updated by Victor Julien about 2 years ago

Can you create (a) SV test case(s)?

Actions #5

Updated by Victor Julien about 2 years ago

Can you test against: Suricata 4.1.5 (no rust), 4.1.5 (with rust) and git master?

Actions #6

Updated by Victor Julien about 2 years ago

  • Status changed from New to Feedback
  • Assignee changed from OISF Dev to Travis Green
  • Target version changed from TBD to 70
Actions #7

Updated by Travis Green almost 2 years ago

Submitted PR for suricata-verify test https://github.com/OISF/suricata-verify/pull/139

Actions #8

Updated by Travis Green almost 2 years ago

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  PATCHED  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-git.latest/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
[29956] 21/10/2019 -- 12:36:03 - (suricata.c:1072) <Notice> (LogVersion) -- This is Suricata version 5.0.0-dev (412ae11ba 2019-10-12) running in USER mode
<snip>
   56  1331 14696 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-git.debug/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
[30428] 21/10/2019 -- 12:41:15 - (suricata.c:1076) <Notice> (LogVersion) -- This is Suricata version 5.0.0-dev (494617bb3 2019-09-12) running in USER mode
<snip>
   42   974 10832 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED 4.1.5 w/rust  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
$ cat /dev/null > ./fast.log && sudo /opt/suricata-4.1.5.rust/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
21/10/2019 -- 12:36:49 - <Notice> - This is Suricata version 4.1.5 RELEASE
<snip>
   42   974 10832 ./fast.log

-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=  UNPATCHED 4.1.5 w/o rust  -=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=-=
cat /dev/null > ./fast.log && sudo /opt/suricata-4.1.5.norust/src/suricata -c /etc/suricata/suricata.testsuri4.yaml -l . -S ~/rules/lateral-rules/lateral.rules -k none -r ./merged.pcap && cat ./fast.log && wc ./fast.log
21/10/2019 -- 12:36:54 - <Notice> - This is Suricata version 4.1.5 RELEASE
   46  1076 11936 ./fast.log

details @ https://gist.github.com/travisbgreen/e3b34e848efbe2fe0dc37183786ce9be

Actions #9

Updated by Victor Julien over 1 year ago

  • Assignee changed from Travis Green to OISF Dev
Actions #10

Updated by Victor Julien about 1 year ago

  • Target version changed from 70 to TBD

I think this works as intended: bind followed by a request. IIRC this was done for Snort compatibility.

I agree it would be nicer to be able to just match on the bind, but I don't want to break existing rules either.

Perhaps we need a new set of keywords where compatibility doesn't have to be taken into account.

Actions #11

Updated by Philippe Antoine 7 months ago

  • Assignee changed from OISF Dev to Shivani Bhardwaj
Actions

Also available in: Atom PDF