8000 Selection of TX variables via RegEx should not be case-sensitive · Issue #2296 · owasp-modsecurity/ModSecurity · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

Selection of TX variables via RegEx should not be case-sensitive #2296

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
michaelgranzow-avi opened this issue Apr 20, 2020 · 5 comments
Closed
Assignees
Labels
3.x Related to ModSecurity version 3.x

Comments

@michaelgranzow-avi
Copy link
Contributor
michaelgranzow-avi commented Apr 20, 2020

Description

Behaviour of ModSec 2.9 and 3.x differs with respect to selection of elements in the 'TX' collection with regular expressions. In 2.9, the RegEx matches without taking case into account, in 3.x the match is case-sensitive.

Logs and dumps

Output of the modsec regression test tool with the first test case in the file below as input:
ModSecurity 3.0.4 - tests
(options are not available -- missing GetOpt)

# File Name Test Name Passed?
--- --------- --------- -------
1 tx-regex-var.json Testing regex selection from TX - case failed!

Test failed. From: tx-regex-var.json.
Test name: Testing regex selection from TX - case.
Reason:
HTTP code mismatch. expecting: 437 got: 200

Debug log:
[1587380363] [] [4] Initializing transaction
[1587380363] [] [4] Transaction context created.
[1587380363] [] [4] Starting phase CONNECTION. (SecRules 0)
[1587380363] [] [9] This phase consists of 0 rule(s).
[1587380363] [] [4] Starting phase URI. (SecRules 0 + 1/2)
[1587380363] [/] [4] Starting phase REQUEST_HEADERS. (SecRules 1)
[1587380363] [/] [9] This phase consists of 1 rule(s).
[1587380363] [/] [4] (Rule: 0) Executing unconditional rule...
[1587380363] [/] [4] Running [independent] (non-disruptive) action: setvar
[1587380363] [/] [8] Saving variable: TX:restricted_headers with value: /name1/name2/
[1587380363] [/] [9] Appending request body: 0 bytes. Limit set to: 0.000000
[1587380363] [/] [4] Starting phase REQUEST_BODY. (SecRules 2)
[1587380363] [/] [9] This phase consists of 1 rule(s).
[1587380363] [/] [4] (Rule: 500065) Executing operator "Rx" with param "^.*$" against REQUEST_HEADERS_NAMES.
[1587380363] [/] [9] T (0) t:lowercase: "host"
[1587380363] [/] [9] Target value: "host" (Variable: REQUEST_HEADERS_NAMES:Host)
[1587380363] [/] [7] Added regex subexpression TX.0: host
[1587380363] [/] [9] Matched vars updated.
[1587380363] [/] [4] Running [independent] (non-disruptive) action: setvar
[1587380363] [/] [8] Saving variable: TX:header_name_host with value: /host/
[1587380363] [/] [9] Saving msg: capture
[1587380363] [/] [9] T (0) t:lowercase: "user-agent"
[1587380363] [/] [9] Target value: "user-agent" (Variable: REQUEST_HEADERS_NAMES:User-Agent)
[1587380363] [/] [7] Added regex subexpression TX.0: user-agent
[1587380363] [/] [9] Matched vars updated.
[1587380363] [/] [4] Running [independent] (non-disruptive) action: setvar
[1587380363] [/] [8] Saving variable: TX:header_name_user-agent with value: /user-agent/
[1587380363] [/] [9] Saving msg: capture
[1587380363] [/] [9] T (0) t:lowercase: "name1"
[1587380363] [/] [9] Target value: "name1" (Variable: REQUEST_HEADERS_NAMES:name1)
[1587380363] [/] [7] Added regex subexpression TX.0: name1
[1587380363] [/] [9] Matched vars updated.
[1587380363] [/] [4] Running [independent] (non-disruptive) action: setvar
[1587380363] [/] [8] Saving variable: TX:header_name_name1 with value: /name1/
[1587380363] [/] [9] Saving msg: capture
[1587380363] [/] [4] Rule returned 1.
[1587380363] [/] [4] Executing chained rule.
[1587380363] [/] [4] (Rule: 0) Executing operator "Within" with param "/name1/name2/" Was: "" against TX:regex(^HEADER_NAME_).
[1587380363] [/] [4] Rule returned 0.
[1587380363] [/] [9] Matched vars cleaned.
[1587380363] [/] [4] Starting phase RESPONSE_HEADERS. (SecRules 3)
[1587380363] [/] [9] This phase consists of 0 rule(s).
[1587380363] [/] [9] Appending response body: 0 bytes. Limit set to: 0.000000
[1587380363] [/] [4] Starting phase RESPONSE_BODY. (SecRules 4)
[1587380363] [/] [4] Response body is disabled, returning... 2
[1587380363] [/] [4] Starting phase LOGGING. (SecRules 5)
[1587380363] [/] [9] This phase consists of 0 rule(s).
[1587380363] [/] [8] Checking if this request is suitable to be saved as an audit log.
[1587380363] [/] [8] Checking if this request is relevant to be part of the audit logs.
[1587380363] [/] [5] Audit log engine was not set.
[1587380363] [/] [8] Request was relevant to be saved. Parts: 4430

Error log:

Audit log:

Ran a total of: 1 regression tests - 1 failed. 0 skipped test(s). 0 disabled test(s).

To Reproduce

Steps to reproduce the behavior:

First test case in the following ModSecurity v3 test file (adapted from CRS rule 920450): Note that setvar uses tx.header_name_foo, but the variable in the chained rule is identified as TX/^HEADER_NAME_/

[
  {
    "enabled":1,
    "version_min":300000,
    "title":"Testing regex selection from TX - case",
    "expected":{
      "http_code": 437
    },
    "client":{
      "ip":"200.249.12.31",
      "port":123
    },
    "request":{
      "headers":{
        "Host":"localhost",
        "User-Agent":"curl/7.38.0",
        "name1": "value1"
      },
      "uri":"/",
      "method":"GET"
    },
    "server":{
      "ip":"200.249.12.31",
      "port":80
    },
    "rules":[
        "SecRuleEngine On",
        "SecAction \"phase:1,setvar:'TX.restricted_headers=/name1/name2/'\"",
        "SecRule REQUEST_HEADERS_NAMES \"^.*$\" \"phase:2,t:none,t:lowercase,setvar:'tx.header_name_%{tx.0}=/%{tx.0}/',deny,status:437,msg:'capture',capture,id:500065,chain\"\n",
        "SecRule TX:/^HEADER_NAME_/ \"@within %{tx.restricted_headers}\" \"setvar:'tx.matched=1'\""
    ]
  },
  {
    "enabled":1,
    "version_min":300000,
    "title":"Testing single element selection from TX - case",
    "expected":{
      "http_code": 437
    },
    "client":{
      "ip":"200.249.12.31",
      "port":123
    },
    "request":{
      "headers":{
        "Host":"localhost",
        "User-Agent":"curl/7.38.0",
        "name1": "value1"
      },
      "uri":"/file.ext2",
      "method":"GET"
    },
    "server":{
      "ip":"200.249.12.31",
      "port":80
    },
    "rules":[
        "SecRuleEngine On",
        "SecAction \"phase:1,setvar:'TX.restricted_extensions=/ext1/ext2/'\"",
        "SecRule REQUEST_BASENAME \"@rx \\.([^.]+)$\" \"phase:2,t:none,t:lowercase,setvar:'tx.extension=/%{tx.1}/',deny,status:437,msg:'capture',capture,id:500065,chain\"\n",
        "SecRule TX:EXTENSION \"@within %{tx.restricted_extensions}\" \"setvar:'tx.matched=1'\""
    ]
  }
]

Expected behavior

There are several collections in ModSec, and they differ regarding case-sensitivity:

  1. TX should be case-insensitive as per legacy behaviour. Note that selection of an individual element is case-insensitive in modsec 3.x already, see second test case in the file, which passes.
  2. REQUEST_HEADERS and RESPONSE_HEADERS are case-insensitive because that's what the HTTP standard says (same for the corresponding NAMES collections).
  3. ARGS is case-sensitive (same for ARGS_GET and ARGS_POST and for the corresponding NAMES collections)
  4. COOKIES (and NAMES) are case-sensitive
  5. FILES ?

Server:

  • ModSecurity v3.0.4 library, no connector needed
  • Ubuntu Linux 16.04.5 LTS

Rule Set:

  • See test file. No official set needed, but this affects all versions of CRS at least since 2017 because the chained rule handling restricted headers assumes the 2.9 behaviour.
@airween
Copy link
Member
airween commented Apr 20, 2020

Note, that fix exists.

@michaelgranzow-avi
Copy link
Contributor Author

Thanks @airween - do you know why it hasn't been merged? It seems to be marked as duplicate, but isn't as the other bug is about full variable name match, not regex match of the variable name.

@airween
Copy link
Member
airween commented Apr 20, 2020

@michaelgranzow-avi sorry, I have no idea :).

Also I don't know, what's the "other" issue, so why is this PR marked as duplicated.

@michaelgranzow-avi
Copy link
Contributor Author

The other issue is this:

#1808

@zimmerle zimmerle self-assigned this Apr 27, 2020
@zimmerle zimmerle added the 3.x Related to ModSecurity version 3.x label Apr 27, 2020
zimmerle added a commit that referenced this issue Nov 30, 2020
This issue was initially reported by @michaelgranzow-avi on #2296.

@airween made an initial attempt to provide a fixed at #2107; As a
consequence of the pull request review - provided by @victorhora,
@zimmerle, and @michaelgranzow-avi - @airween made a second attempt
at #2297. After reviewing by @martinhsv, @zimmerle, I have absorbed
the essential pieces from @airween patch into this one.

This patch differs from @airween's because @airween's patches were
partially working: Key exclusions with regex weren't covered, same
for anchored variables (e.g. ARGS). During the review, I have
highlighted the importance of having elementary test cases. A simple
test case on ARGS could spot the issue. Since that is an important
fix, I don't want to hold this for one more review cycle; therefore,
I am committing the fix myself.

Thank you all involved in the solution of this very own issue.
@zimmerle
Copy link
Contributor

fixed by 910a187

zimmerle added a commit that referenced this issue Dec 10, 2020
This issue was initially reported by @michaelgranzow-avi on #2296.

@airween made an initial attempt to provide a fixed at #2107; As a
consequence of the pull request review - provided by @victorhora,
@zimmerle, and @michaelgranzow-avi - @airween made a second attempt
at #2297. After reviewing by @martinhsv, @zimmerle, I have absorbed
the essential pieces from @airween patch into this one.

This patch differs from @airween's because @airween's patches were
partially working: Key exclusions with regex weren't covered, same
for anchored variables (e.g. ARGS). During the review, I have
highlighted the importance of having elementary test cases. A simple
test case on ARGS could spot the issue. Since that is an important
fix, I don't want to hold this for one more review cycle; therefore,
I am committing the fix myself.

Thank you all involved in the solution of this very own issue.
vladbukin pushed a commit to vladbukin/ModSecurity that referenced this issue Apr 12, 2022
This issue was initially reported by @michaelgranzow-avi on owasp-modsecurity#2296.

@airween made an initial attempt to provide a fixed at owasp-modsecurity#2107; As a
consequence of the pull request review - provided by @victorhora,
@zimmerle, and @michaelgranzow-avi - @airween made a second attempt
at owasp-modsecurity#2297. After reviewing by @martinhsv, @zimmerle, I have absorbed
the essential pieces from @airween patch into this one.

This patch differs from @airween's because @airween's patches were
partially working: Key exclusions with regex weren't covered, same
for anchored variables (e.g. ARGS). During the review, I have
highlighted the importance of having elementary test cases. A simple
test case on ARGS could spot the issue. Since that is an important
fix, I don't want to hold this for one more review cycle; therefore,
I am committing the fix myself.

Thank you all involved in the solution of this very own issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.x Related to ModSecurity version 3.x
Projects
None yet
Development

No branches or pull requests

3081
3 participants
0