detect/firewall: reject unsatisfiable app-layer-state rules

An app-layer-state comparison that can never hold - mode '>' at or beyond
the completion state, or mode '<' at the starting state - can never match.
Both are fatal at rule load, so reject them at parsing.

The LTE (<hook) form at the first state (progress 0) is accepted and is
equivalent to the plain hook form: there are no prior states to auto-accept.

Derive the completion state from the direction the keyword resolved in
instead of the rule's flags, and document the new load time rejections.

Ticket: #8944.
This commit is contained in:
Victor Julien
2026-10-03 14:09:36 +00:00
parent 59d08a717e
commit 9419f15779
4 changed files with 35 additions and 0 deletions
@@ -417,3 +417,6 @@ defining ``accept:tx`` as a global default policy will fail to start Suricata,
because ``packet`` policies do not accept ``tx``.
Cover such hooks with a more specific setting so the incompatible default never
reaches them.
A ``<`` hook rule at the protocol's first state (progress 0) is accepted and is
equivalent to the plain hook form, as there are no prior states to auto-accept.
+9
View File
@@ -213,3 +213,12 @@ Examples::
app-layer-state:request_headers;
app-layer-state:>request_body;
A comparison that can never hold is rejected at rule load:
* ``>`` with a state at or beyond the protocol's completion state;
* ``<`` with the protocol's first state (progress 0).
.. note:: The keyword only resolves on protocols whose parser registers a
top-level state table (``get_state_id_by_name``). The ``http2`` and
``doh2`` parsers do not, so ``app-layer-state`` cannot be used on them.
+6
View File
@@ -162,6 +162,12 @@ per protocol specific hooks.
More details can be found in :ref:`rule-hooks`.
.. note::
A ``<`` hook at the protocol's first state (progress 0) is accepted and is
equivalent to the plain hook form: there are no prior states to auto-accept
(``http1:<request_started`` equals ``http1:request_started``).
.. note::
While developed for the firewall usecase, these hooks can be used in IDS/IPS rules as well.
+17
View File
@@ -199,6 +199,7 @@ static int DetectAppLayerStateSetup(DetectEngineCtx *de_ctx, Signature *s, const
}
int progress = 0;
uint8_t dir_flag = STREAM_TOSERVER;
const char *h = arg;
const int progress_ts =
AppLayerParserGetStateIdByName(IPPROTO_TCP /* TODO */, s->alproto, h, STREAM_TOSERVER);
@@ -213,8 +214,24 @@ static int DetectAppLayerStateSetup(DetectEngineCtx *de_ctx, Signature *s, const
}
s->flags |= SIG_FLAG_TOCLIENT;
progress = progress_tc;
dir_flag = STREAM_TOCLIENT;
}
const uint8_t direction = dir_flag;
const uint8_t end_state = AppLayerParserGetStateProgressCompletionStatus(s->alproto, direction);
/* the tx progress can never exceed the completion state */
if (mode > 0 && progress >= end_state) {
SCLogError("app-layer-state: state '%s' is not below the completion state for mode '>'", h);
return -1;
}
/* the progress can never be below the starting state */
if (mode < 0 && progress == 0) {
SCLogError("app-layer-state: state '%s' is the starting state, it can never match for "
"mode '<'",
h);
return -1;
}
DetectAppLayerStateData *data = SCCalloc(1, sizeof(*data));
if (data == NULL)
return -1;