Skip to content

Commit

Permalink
detect: delay tx cleanup in some edge case
Browse files Browse the repository at this point in the history
Ticket: 7552

f->sgh_toserver may be NULL but because FLOW_SGH_TOSERVER is unset
and thus, we want to delay cleanup until detection has really been
run with the right signature group head.

This may happen for a rule using
`alert tcp any any -> any any` and
a app-layer keyword to client
with a app-layer supporting both udp and tcp
with stream.midstream=true
and with the first packet of a flow being a server response

In this case, we swap the flow and reset its signature group heads
  • Loading branch information
catenacyber committed Feb 25, 2025
1 parent d9b8b08 commit 43c2134
Show file tree
Hide file tree
Showing 4 changed files with 7 additions and 2 deletions.
1 change: 1 addition & 0 deletions src/app-layer-htp.c
Original file line number Diff line number Diff line change
Expand Up @@ -5628,6 +5628,7 @@ static int HTPParserTest25(void)
f->protoctx = &ssn;
f->proto = IPPROTO_TCP;
f->alproto = ALPROTO_HTTP1;
f->flags |= FLOW_SGH_TOCLIENT | FLOW_SGH_TOSERVER;

const char *str = "GET / HTTP/1.1\r\nHost: www.google.com\r\nUser-Agent: Suricata/1.0\r\n\r\n";
int r = AppLayerParserParse(NULL, alp_tctx, f, ALPROTO_HTTP1, STREAM_TOSERVER | STREAM_START,
Expand Down
1 change: 1 addition & 0 deletions src/app-layer-ike.c
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@ static int IkeParserTest(void)
f.proto = IPPROTO_UDP;
f.protomap = FlowGetProtoMapping(f.proto);
f.alproto = ALPROTO_IKE;
f.flags |= FLOW_SGH_TOCLIENT | FLOW_SGH_TOSERVER;

StreamTcpInitConfig(true);

Expand Down
6 changes: 4 additions & 2 deletions src/app-layer-parser.c
Original file line number Diff line number Diff line change
Expand Up @@ -957,7 +957,8 @@ void AppLayerParserTransactionsCleanup(Flow *f, const uint8_t pkt_dir)
}

if (txd && has_tx_detect_flags) {
if (!IS_DISRUPTED(ts_disrupt_flags) && f->sgh_toserver != NULL) {
if (!IS_DISRUPTED(ts_disrupt_flags) &&
(f->sgh_toserver != NULL || (f->flags & FLOW_SGH_TOSERVER) == 0)) {
uint64_t detect_flags_ts = AppLayerParserGetTxDetectFlags(txd, STREAM_TOSERVER);
if (!(detect_flags_ts &
(APP_LAYER_TX_INSPECTED_FLAG | APP_LAYER_TX_SKIP_INSPECT_FLAG))) {
Expand All @@ -966,7 +967,8 @@ void AppLayerParserTransactionsCleanup(Flow *f, const uint8_t pkt_dir)
tx_skipped = true;
}
}
if (!IS_DISRUPTED(tc_disrupt_flags) && f->sgh_toclient != NULL) {
if (!IS_DISRUPTED(tc_disrupt_flags) &&
(f->sgh_toclient != NULL || (f->flags & FLOW_SGH_TOCLIENT) == 0)) {
uint64_t detect_flags_tc = AppLayerParserGetTxDetectFlags(txd, STREAM_TOCLIENT);
if (!(detect_flags_tc &
(APP_LAYER_TX_INSPECTED_FLAG | APP_LAYER_TX_SKIP_INSPECT_FLAG))) {
Expand Down
1 change: 1 addition & 0 deletions src/app-layer-smb.c
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,7 @@ static int SMBParserTxCleanupTest(void)
f->protoctx = &ssn;
f->proto = IPPROTO_TCP;
f->alproto = ALPROTO_SMB;
f->flags |= FLOW_SGH_TOCLIENT | FLOW_SGH_TOSERVER;

char req_str[] ="\x00\x00\x00\x79\xfe\x53\x4d\x42\x40\x00\x01\x00\x00\x00\x00\x00" \
"\x05\x00\xe0\x1e\x10\x00\x00\x00\x00\x00\x00\x00\x0b\x00\x00\x00" \
Expand Down

0 comments on commit 43c2134

Please sign in to comment.