Project

General

Profile

Actions

Bug #9224

open
AP LS

firewall: payload keywords on a packet scoped hook aborts instead of rejecting the rule

Bug #9224: firewall: payload keywords on a packet scoped hook aborts instead of rejecting the rule

Added by Aneesh Patel 18 days ago. Updated 12 days ago.

Status:
Assigned
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
medium
Difficulty:
Label:
Needs Suricata-Verify test

Description

Firewall mode: payload keyword (`content`/`pcre`/…) on a packet hook aborts via `BUG_ON` in `DetectRuleSetTable` instead of rejecting the rule

Version: Suricata 8.0.6

Summary

A firewall rule that uses a packet hook (`tcp:all`, `tcp:flow_start`, `tcp:pre_flow`, `tcp:pre_stream`, and the equivalents for `udp`/`ip`/`icmp`) together with a payload (PMATCH) keyword (`content`, `pcre`, `byte_test`, `byte_jump`, `isdataat`) crashes the engine during rule loading with an assertion failure. A single such rule aborts the whole engine start / `-T` run — it is not reported as an invalid rule.

Steps to reproduce

Minimal rule file (`fw.rules`):

drop:packet tcp:all any any -> any any (content:"x"; sid:1;)

Run in firewall mode with config-test:

suricata -T -c suricata.yaml --firewall-rules-exclusive fw.rules

Actual result

suricata: detect-parse.c:2797: DetectRuleSetTable: Assertion `!(1)' failed. Error: suricata: stacktrace:sig 6:gsignal;abort;__assert_fail_base;__assert_fail; SigInitHelper;DetectFirewallRuleAppendNew;DetectLoadSigFile;SigLoadSignatures; PostConfLoadedDetectSetup;SuricataInit;main;__libc_start_main;_start

Process exits with SIGABRT (exit code 134).

Expected result

The rule should be rejected with an `SCLogError` and a non-zero return (treated as an invalid signature), the same way other illegal firewall rule shapes are — not an `abort()`.

Scope

- Reproduces on all four packet hooks (`all`, `flow_start`, `pre_flow`, `pre_stream`) and all tested packet protos (`tcp`, `udp`, `ip`, `icmp`) — 16/16.
- Independent of content modifiers: bare `content`, `content; depth`, `content; offset`, `content; startswith`, `content; nocase`, `content; rawbytes` all abort, as do `pcre`, `byte_test`, `byte_jump`, `isdataat`.
- Firewall mode only. The same payload keyword in a normal (IDS/IPS) rule loads fine; `dsize`, `ttl`, `tcp.flags`, `itype`, `ipopts`, `flow:to_server` and other non-PMATCH packet keywords on a packet-hook firewall rule also load fine.
- Payload keywords on an app hook (e.g. `drop:flow tls:client_hello (tls.sni; content:"x"; sid:1;)`) load fine.

Root cause

`DetectRuleSetTable()` (`src/detect-parse.c:2782`) selects the detect table from `s->type`. The firewall branch handles only two signature types and asserts on the rest:

```c
if (s->flags & SIG_FLAG_FIREWALL) {
if (s->type SIG_TYPE_PKT) { ... }
else if (s->type SIG_TYPE_APP_TX) { ... }
else { BUG_ON(1); } /* detect-parse.c:2797 */
}
```

A packet-hook rule carrying a PMATCH match is classified as neither of those. The call sequence in SigValidateConsolidate() is SigConsolidateTcpBuffer() → SignatureSetType() → DetectRuleSetTable() (src/detect-parse.c:3162-3165):

- SigConsolidateTcpBuffer() (src/detect-parse.c:3048) sets SIG_FLAG_REQUIRE_STREAM for a PMATCH content on TCP (adding SIG_FLAG_REQUIRE_PACKET only when depth/offset is present).
- SignatureSetType()'s has_pmatch branch (src/detect-engine-build.c:1683-1693) then yields SIG_TYPE_STREAM (TCP, stream-only) or SIG_TYPE_PKT_STREAM (TCP with depth/offset, or UDP/IP/ICMP).
- Neither type is handled by the firewall branch of DetectRuleSetTable() → BUG_ON(1).

The non-firewall branch of the same function has an else that maps any type to DETECT_TABLE_PACKET_TD, which is why only firewall-mode rules are affected. SigValidatePacketStream() (src/detect-parse.c:3000) does not guard this case: it runs before SigConsolidateTcpBuffer() assigns the stream flag, and it only triggers when both SIG_FLAG_REQUIRE_PACKET and SIG_FLAG_REQUIRE_STREAM are set, so the stream-only classification passes through unguarded.

SIG_TYPE_STREAM and SIG_TYPE_PKT_STREAM are otherwise first-class types (handled in src/detect-engine-analyzer.c and in the signature-property table in src/detect-engine.c), so this appears to be a missing-case assertion rather than an intended rejection — BUG_ON marks an assumed-unreachable state, whereas invalid firewall rules elsewhere are rejected with SCLogError + a non-zero return.

Suggested fix

In the firewall branch of DetectRuleSetTable(), replace the BUG_ON(1) fallthrough with a graceful rejection path for a payload/stream signature on a packet hook (e.g. return an error that SigInitHelper/DetectFirewallRuleAppendNew surfaces as "payload keywords are not supported on a packet hook; use an app-layer hook"), or extend the branch to map SIG_TYPE_STREAM / SIG_TYPE_PKT_STREAM to an appropriate table. Either way, a malformed rule should not abort() the process.

Actions

Also available in: PDF Atom