Project

General

Profile

Actions

Bug #9086

open
JL JL

detect/isdataat: negated relative isdataat with a byte variable is rewritten to endswith when the variable's local id is 1

Bug #9086: detect/isdataat: negated relative isdataat with a byte variable is rewritten to endswith when the variable's local id is 1

Added by Jeff Lucovsky 1 day ago.

Status:
New
Priority:
Normal
Assignee:
Target version:
Affected Versions:
Effort:
Difficulty:
Label:
Needs backport

Description

A rule that ends a content match with isdataat:!<var>,relative silently loses that check when the byte variable happens to have local id 1. The rule loads without a warning and then matches on something else than what was written.

Cause

DetectIsdataatSetup() uses one field for two things. For a literal offset, idad->dataat holds the byte count. For a variable it holds the variable's local id, and the keyword is flagged ISDATAAT_OFFSET_VAR (src/detect-isdataat.c:354-355 on main). A few lines later the 'ends with' rewrite (src/detect-isdataat.c:361-370) tests idad->dataat == 1 together with the relative and negated flags, but never looks at ISDATAAT_OFFSET_VAR. A variable with local id 1 passes that test, so the isdataat is freed and the preceding content gets DETECT_CONTENT_ENDS_WITH instead.

The rewrite is meant for the literal isdataat:!1,relative only, where "no byte follows" and "the content ends the buffer" are the same thing. The check predates variable offsets: the rewrite came with 842dfbc3f8 (2017-03), variable offsets with #2250 (2017-10).

Local id 1 is what the second byte_extract (or second byte_math) in a rule gets, so any rule with two of them that uses the second in a negated relative isdataat is affected.

Reproducer

alert http any any -> any any (msg:"isdataat with variable b, local id 1"; flow:established,to_server; http.uri; content:"/v/"; byte_extract:1,0,a,relative,string,dec; byte_extract:1,1,b,relative,string,dec; content:"END"; isdataat:!b,relative; sid:1;)
alert http any any -> any any (msg:"literal, legitimately rewritten"; flow:established,to_server; http.uri; content:"/v/"; content:"END"; isdataat:!1,relative; sid:2;)

No traffic is needed to see it. With --engine-analysis --set engine-analysis.rules=yes, rules.json lists these matches for the http_uri engine on main (d996d85dcf):

sid 1: content "/v/", byte_extract, byte_extract, content "END" ends_with=true
sid 2: content "/v/", content "END" ends_with=true

sid 1 has no isdataat left, and its last content carries the same endswith as the literal rule.

With a request for /v/1/5/ENDxx: a = 1, b = 5, two bytes follow "END", which is fewer than 5, so sid 1 should alert. It does not: 0 alerts on main. With the fix sid 1 alerts once and sid 2, correctly, does not.

Impact

False negatives. The rewritten rule only matches when the content ends the buffer, which is a subset of what the rule asked for. I could not construct a false positive: when the content does end the buffer, the negated check is also true for any variable value of 1 or more.

Fix

Only rewrite when ISDATAAT_OFFSET_VAR is clear:

if (prev_pm != NULL && prev_pm->type == DETECT_CONTENT && idad->dataat == 1 &&
        (idad->flags & (ISDATAAT_RELATIVE | ISDATAAT_NEGATED | ISDATAAT_OFFSET_VAR)) ==
                (ISDATAAT_RELATIVE | ISDATAAT_NEGATED)) {

The same code is at src/detect-isdataat.c:363-372 in main-8.0.x, so the backport is a clean pick.

Relation to #7801

Found while working on #7801, which numbers byte variables per signature, across byte_extract and byte_math. That puts more variables on local id 1, so #7801 should land after this. The defect itself does not depend on #7801 and reproduces on main today.


Related issues 1 (1 open0 closed)

Related to Suricata - Feature #7801: rules: support multi-buffer byte variablesIn ReviewJeff LucovskyActions

JL Updated by Jeff Lucovsky 1 day ago Actions #1

  • Related to Feature #7801: rules: support multi-buffer byte variables added
Actions

Also available in: PDF Atom