Bug #9224
openfirewall: payload keywords on a packet scoped hook aborts instead of rejecting the rule
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.
VJ Updated by Victor Julien 15 days ago
- Subject changed from Payload keywords on a packet scoped hook aborts instead of rejecting the rule to firewall: payload keywords on a packet scoped hook aborts instead of rejecting the rule
- Status changed from New to Assigned
- Assignee set to Lukas Sismis
- Target version changed from TBD to 9.0.0-beta1
@Lukas Sismis can you also look at if this needs a backport?
VJ Updated by Victor Julien 12 days ago
It seems to me the rule should work, and not error out. But needs a bit more analysis.