Bug #2224
openNegated http_* match returns false if buffer not populated
Description
If a rule has a negated content match for 'http_user_agent' buffer but the http_user_agent buffer isn't populated (i.e. the HTTP traffic doesn't have a "User-Agent" header), the negated content match will return false when it should be true. Example:
HTTP traffic:
GET /nouser-agent.html HTTP/1.1 Accept: */*
Rule:
alert http any any -> any any (msg:"User-Agent"; flow:established,to_server; content:"user-agent"; http_uri; content:!"doesnotexist"; http_user_agent; priority:2; sid:8675309;)
The above traffic does not have a User-Agent buffer so any negated content match in the http_user_agent buffer should return true. However, the above rule does not alert (unless the http_user_agent content match is removed).
Tested on Suricata 4.0.0, 3.2.3, 2.9.0, etc. This behavior applies to other http_* buffers too. e.g. http_host:
alert http any any -> any any (msg:"User-Agent"; flow:established,to_server; content:"user-agent"; http_uri; content:!"doesnotexist"; http_host; priority:2; sid:8675308;)
Maybe this behavior is "as designed" ... if so, can this bug report be turned in to a feature request?
Files
Updated by Andreas Herz about 7 years ago
- Assignee set to OISF Dev
- Target version set to TBD
Updated by Philippe Antoine over 5 years ago
- Related to Bug #2479: http_cookie negation fails if no cookie in traffic added
Updated by Andreas Herz over 5 years ago
Philippe are you looking into those two related issues so I can assign them to you :)?
Updated by Victor Julien over 5 years ago
This is not really a bug. It's more a design decisions about to how to deal with negation on buffers we don't see. Right now won't match as the rule is only evaluated if the buffer is present.
Updated by Philippe Antoine over 5 years ago
So, the feature request would be to have a new negate keyword that includes absence ?
Updated by Jason Ish about 4 years ago
- Related to Task #4097: Suricon 2020 brainstorm added
Updated by Brandon Murphy over 3 years ago
Just as an FYI - this issue/design/whatever continues to cause False Negatives on a pretty regular basis. If there is some sort of unofficial "vote" for tickets which need attention, consider this my vote.
Updated by Victor Julien over 2 years ago
@Brandon Murphy could you share some test cases to show how this causes FN's?
Updated by Philippe Antoine over 1 year ago
Digging a bit into this.
If we want this behavior, we need to patch DetectEngineInspectBufferGeneric (and others) so that it does not bail out early with if (unlikely(buffer NULL)) but checks if engine->smd->type DETECT_CONTENT and cd->flags & DETECT_CONTENT_NEGATED to return DETECT_ENGINE_INSPECT_SIG_MATCH
But it only works if we have another buffer to pass the pre filter step...
Updated by Philippe Antoine over 1 year ago
- Blocked by Bug #6025: detect: allow bsize 0 for existing empty buffers added
Updated by Philippe Antoine over 1 year ago
https://redmine.openinfosecfoundation.org/issues/6025 simple patch is a prerequisite for this.
This will help distinguish absent buffers (matching negated content but not size 0) from empty buffers (matching both bsize 0 and negated content)
Updated by Philippe Antoine over 1 year ago
Posting my patch here waiting for #6025 merge
diff --git a/src/detect-engine.c b/src/detect-engine.c
index ce2d319d5..0c64b1e4b 100644
--- a/src/detect-engine.c
+++ b/src/detect-engine.c
@@ -2050,6 +2050,12 @@ uint8_t DetectEngineInspectBufferGeneric(DetectEngineCtx *de_ctx, DetectEngineTh
const InspectionBuffer *buffer = engine->v2.GetData(det_ctx, transforms,
f, flags, txv, list_id);
if (unlikely(buffer == NULL)) {
+ if (engine->smd->type == DETECT_CONTENT) {
+ DetectContentData *cd = (DetectContentData *)engine->smd->ctx;
+ if (cd->flags & DETECT_CONTENT_NEGATED) {
+ return DETECT_ENGINE_INSPECT_SIG_MATCH;
+ }
+ }
return eof ? DETECT_ENGINE_INSPECT_SIG_CANT_MATCH :
DETECT_ENGINE_INSPECT_SIG_NO_MATCH;
}
Updated by Brandon Murphy over 1 year ago
Victor Julien wrote in #note-8:
@Brandon Murphy could you share some test cases to show how this causes FN's?
I somehow missed this, @Philippe Antoine do you still need test cases?
Updated by Philippe Antoine over 1 year ago
Brandon, yes please, your tests cases are welcome
Updated by Brandon Murphy over 1 year ago
- File no_accept.pcap no_accept.pcap added
- File no_user_agent.pcap no_user_agent.pcap added
- File no_accept_language.pcap no_accept_language.pcap added
- File no_accept_encoding.pcap no_accept_encoding.pcap added
- File no_referer.pcap no_referer.pcap added
- File no_content_type.pcap no_content_type.pcap added
- File no_connection.pcap no_connection.pcap added
- File all_headers.pcap all_headers.pcap added
Basic rules for multiple HTTP request rules and associated pcaps for testing.
# Test to ensure it works without a negated content # This signature should alert with _any_ pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"TP test for URI"; flow:established,to_server; http.uri; bsize:1; content:"/"; sid:1;) # Test to prove an FN when UA is not included # This signature will not alert (FN) on pcaps where the UA is not included in the HTTP Request # test pcap = no_user_agent.pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for User-Agent"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.user_agent; content:!"example"; sid:2;) # Test to prove FN when ACCEPT is not included # This signature will not alert (FN) on pcaps where the Accept is not included in the HTTP Request # test pcap = no_accept.pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Accept"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.accept; content:!"example"; sid:3;) # Test to prove FN when Accept-Language is not included # This signature will not alert (FN) on pcaps where the Accept-Language is not included in the HTTP Request # test pcap = no_accept_language.pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Accept-Language"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.accept_lang; content:!"example"; sid:4;) # Test to prove FN when Accept-Encoding is not included # This signature will not alert (FN) on pcaps where the Accept-Encoding is not included in the HTTP Request # test pcap = no_accept_encoding.pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Accept-Encoding"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.accept_enc; content:!"example"; sid:5;) # Test to prove FN when Referer is not included alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Referer"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.referer; content:!"example"; sid:6;) # Test to prove FN when Connection is not included alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Connection"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.connection; content:!"example"; sid:7;) # Test to prove FN when Content-Type is not included # This signature will not alert (FN) on pcaps where the Content-Type is not included in the HTTP Request # test pcap = no_content_type.pcap alert http $HOME_NET any -> $EXTERNAL_NET any (msg:"FN test for Content-Type"; flow:established,to_server; http.uri; bsize:1; content:"/"; http.content_type; content:!"example"; sid:8;)
Output of "no_referer.pcap" - notice that sid:6 does not fire
05/03/2023-14:18:45.426064 [**] [1:1:0] TP test for URI [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:2:0] FN test for User-Agent [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:3:0] FN test for Accept [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:4:0] FN test for Accept-Language [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:5:0] FN test for Accept-Encoding [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:7:0] FN test for Connection [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
05/03/2023-14:18:45.426064 [**] [1:8:0] FN test for Content-Type [**] [Classification: (null)] [Priority: 3] {TCP} 192.168.244.96:54222 -> 12.16.121.171:80
Updated by Philippe Antoine about 1 year ago
- Assignee changed from OISF Dev to Philippe Antoine
- Target version changed from TBD to 8.0.0-beta1
Updated by Philippe Antoine 12 months ago
- Status changed from New to In Review
Updated by Philippe Antoine 10 months ago
- Status changed from In Review to In Progress
Today's status : https://github.com/OISF/suricata/pull/10140
support all "sticky buffers" not just DetectEngineInspectBufferGeneric
Updated by Philippe Antoine 9 months ago
- Blocked by Optimization #6575: detect/multi-buffer: use single definition of struct PrefilterMpmKrb5Name added
Updated by Philippe Antoine 9 months ago
- Status changed from In Progress to In Review
https://github.com/OISF/suricata/pull/10334 supports all, but the commits for #6575 should be merged first in their own PR
Updated by Victor Julien 5 months ago
- Blocks Story #7124: rules: improve rule language added
Updated by Victor Julien about 1 month ago
- Has duplicate Feature #7322: ability to negate the existence of fields via buffer negation added