]> git.ipfire.org Git - thirdparty/suricata.git/commitdiff
dcerpc: don't reuse completed tx
authorVictor Julien <vjulien@oisf.net>
Wed, 31 Jul 2024 11:58:29 +0000 (13:58 +0200)
committerVictor Julien <victor@inliniac.net>
Tue, 24 Sep 2024 04:56:24 +0000 (06:56 +0200)
In the DCERPC over TCP pcap, logging and rule matching is disrupted by adding a simple rule:

        alert tcp any any -> any any (flow:to_server,established; \
                dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
                dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
                content:!"|00|"; within:100; distance:97; sid:1; rev:1; )

Works: alert + 3 dcerpc records.

But when adding a trivial rule:

        alert tcp any any -> any any (flow:to_server,established; \
                dce_iface:5d2b62aa-ee0a-4a95-91ae-b064fdb471fc; dce_opnum:1; \
                dce_stub_data; content:"|42 77 4E 6F 64 65 49 50 2E 65 78 65 20|"; \
                content:!"|00|"; within:100; distance:97; sid:1; rev:1; )
        alert tcp any any -> any any (dsize:3; sid:2; rev:1; )

The alert for sid:1 disappears and also there is one dcerpc event less.

In the single rule case we can aggressively free the transactions, as there
is only an sgh in the toserver direction.

This means that when we encounter the 2nd REQUEST, the first 2 transactions
have already been processed and freed. So for the 2nd REQUEST we open a new
TX and run inspection and logging on it.

When the 2nd rule is added, it adds toclient sgh as well. This means that we
will now slightly delay the freeing of the transactions.

As a consequence we still have the TX for the first REQUEST when the 2nd REQUEST
is parsed. This leads to the 2nd REQUEST re-using the TX. Since the TX is
already marked as inspected, it means the toserver rule now no longer matches.
Also we're not logging this TX correctly now.

This commit fixes the issue by not "finding" a TX that as already been
marked complete in the search direction.

Bug #7187.

(cherry picked from commit 65392c02f5632d7a8faf30285fb8f5a946cbe9a4)

rust/src/dcerpc/dcerpc.rs

index 62b2e1164eff17d707a1acdd174fc258fa9a82ad..345d4665662463ef227822e0ec902d0f65a4c2a9 100644 (file)
@@ -519,12 +519,18 @@ impl DCERPCState {
             if found {
                 match dir {
                     Direction::ToServer => {
+                        if tx.req_done || tx.req_lost {
+                            continue;
+                        }
                         let resp_cmd = get_resp_type_for_req(cmd);
                         if resp_cmd != tx.resp_cmd {
                             continue;
                         }
                     }
                     Direction::ToClient => {
+                        if tx.resp_done || tx.resp_lost {
+                            continue;
+                        }
                         let req_cmd = get_req_type_for_resp(cmd);
                         if req_cmd != tx.req_cmd {
                             continue;