From 79aa4861e0a7f729c9d827770abe21dce2260b0a Mon Sep 17 00:00:00 2001 From: Victor Julien Date: Fri, 6 Sep 2024 13:14:48 +0200 Subject: [PATCH] detect/app-layer-proto: don't run detection on ALPROTO_UNKNOWN The `app-layer-protocol` keyword inconsistently checks whether the alproto is ALPROTO_UNKNOWN. In the regular match function it isn't checked, in the prefilter function its checked for all but the "either" mode. This leads to false positives for negated matching, as an expression like "!tls" will match if checked against ALPROTO_UNKNOWN. This patch adds the checking everywhere. The keyword returns no match as long as the alproto is ALPROTO_UNKNOWN. Bug: #7241. --- src/detect-app-layer-protocol.c | 20 ++++++++++++++++++-- 1 file changed, 18 insertions(+), 2 deletions(-) diff --git a/src/detect-app-layer-protocol.c b/src/detect-app-layer-protocol.c index 938b96a5b4..c706e5567b 100644 --- a/src/detect-app-layer-protocol.c +++ b/src/detect-app-layer-protocol.c @@ -78,24 +78,38 @@ static int DetectAppLayerProtocolPacketMatch( switch (data->mode) { case DETECT_ALPROTO_DIRECTION: if (p->flowflags & FLOW_PKT_TOSERVER) { + if (f->alproto_ts == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_ts); } else { + if (f->alproto_tc == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_tc); } break; case DETECT_ALPROTO_ORIG: + if (f->alproto_orig == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_orig); break; case DETECT_ALPROTO_FINAL: + if (f->alproto == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto); break; case DETECT_ALPROTO_TOSERVER: + if (f->alproto_ts == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_ts); break; case DETECT_ALPROTO_TOCLIENT: + if (f->alproto_tc == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_tc); break; case DETECT_ALPROTO_EITHER: + if (f->alproto_ts == ALPROTO_UNKNOWN && f->alproto_tc == ALPROTO_UNKNOWN) + SCReturnInt(0); r = AppProtoEquals(data->alproto, f->alproto_tc) || AppProtoEquals(data->alproto, f->alproto_ts); break; @@ -279,9 +293,11 @@ PrefilterPacketAppProtoMatch(DetectEngineThreadCtx *det_ctx, Packet *p, const vo case DETECT_ALPROTO_EITHER: // check if either protocol toclient or toserver matches // the one in the signature ctx - if (AppProtoEquals(ctx->v1.u16[0], f->alproto_tc) ^ negated) { + if (f->alproto_tc != ALPROTO_UNKNOWN && + AppProtoEquals(ctx->v1.u16[0], f->alproto_tc) ^ negated) { PrefilterAddSids(&det_ctx->pmq, ctx->sigs_array, ctx->sigs_cnt); - } else if (AppProtoEquals(ctx->v1.u16[0], f->alproto_ts) ^ negated) { + } else if (f->alproto_ts != ALPROTO_UNKNOWN && + AppProtoEquals(ctx->v1.u16[0], f->alproto_ts) ^ negated) { PrefilterAddSids(&det_ctx->pmq, ctx->sigs_array, ctx->sigs_cnt); } // We return right away to avoid calling PrefilterAddSids again