Skip to content
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

[AppSec] - SecLang rule behaving different in 1.6.4 #3350

Open
victoredvardsson opened this issue Nov 29, 2024 · 8 comments
Open

[AppSec] - SecLang rule behaving different in 1.6.4 #3350

victoredvardsson opened this issue Nov 29, 2024 · 8 comments
Labels

Comments

@victoredvardsson
Copy link
Contributor

victoredvardsson commented Nov 29, 2024

What happened?

One rule that worked prior in 1.6.3 suddenly started to produce false positives. Especially when working with elementor plugin in wordpress.

SecRule REQUEST_METHOD                          "@rx POST"                                          "id:500073,phase:2,t:none,chain,deny,log,msg: 'DEBUG'
SecRule REQUEST_URI                             "@pm /demo.php"          "t:none,t:lowercase,t:urldecode,chain"
SecRule &REQUEST_HEADERS:Referer                "@eq 0"                                             "t:none,t:lowercase"

This is the request (copied from elasticsearch), and it should not match since the url does not contain any of the specified phrases in SecRule REQUEST_URI

  "url.path": [
    "/wp-admin/admin.php?page=wpcode-snippet-manager&snippet_id=24796"
  ],
  "url.path.text": [
    "/wp-admin/admin.php?page=wpcode-snippet-manager&snippet_id=24796"
  ],
@victoredvardsson victoredvardsson added the kind/bug Something isn't working label Nov 29, 2024
Copy link

@victoredvardsson: Thanks for opening an issue, it is currently awaiting triage.

In the meantime, you can:

  1. Check Crowdsec Documentation to see if your issue can be self resolved.
  2. You can also join our Discord.
  3. Check Releases to make sure your agent is on the latest version.
Details

I am a bot created to help the crowdsecurity developers manage community feedback and contributions. You can check out my manifest file to understand my behavior and what I can do. If you want to use this for your project, you can check out the BirthdayResearch/oss-governance-bot repository.

@victoredvardsson
Copy link
Contributor Author

We ended up rolling back to 1.6.3 since we got a lot of reports from customers. Can try to post some more examples later if needed.

@blotus
Copy link
Member

blotus commented Nov 29, 2024

Hello,

Thanks for the report.

I'm a bit confused: you say the rule is triggering a false positive, but according to your example requests, the rule does not match (you get a 404) ?
Is that a typo in the example requests or did I misunderstood something ?

@victoredvardsson
Copy link
Contributor Author

Hello, bad example from me. I will try to get a better one. But anyways after revert to 1.6.3 everything is working as expected for our customers again.

@victoredvardsson
Copy link
Contributor Author

victoredvardsson commented Nov 29, 2024

Here is another rule that also should not react to this request. Since it does not contain /demo.php

# Rule 580300
SecRule REQUEST_METHOD                          "@rx POST"                                          "id:580300,phase:2,t:none,chain,deny,log,msg:DEBUG'"
SecRule REQUEST_URI                             "@contains /demo.php"                "t:none,t:lowercase,t:urldecode,chain"
SecRule ARGS:role                               "@contains administrator"                           "t:none,t:lowercase,t:urldecode”
 - Context  :
╭───────────────┬──────────────────────────────────────────────────────────────╮
│      Key      │                             Value                            │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ host          │ example.domain                                         │
│ id            │ 580300                                                       │
│ matched_zones │ REQUEST_METHOD                                               │
│ matched_zones │ ARGS.role                                                    │
│ method        │ POST                                                         │
│ msg           │ Wordpress - unknown Administrator change    │
│               │ attempt                                                      │
│ name          │ native_rule:580300                                           │
│ uri           │ /wp-admin/user-edit.php                                      │
╰───────────────┴──────────────────────────────────────────────────────────────╯

@buixor
Copy link
Contributor

buixor commented Nov 29, 2024

Hello,

Thanks for the update ! I see what is the bug and what is the root cause (our deduplication of rules that we added in 1.6.4).

We are looking for a proper fix and will keep you posted (1.6.5 most likely). For now it's safer to stick to 1.6.3 if you're using such native mod_sec rules.

Sorry for the inconvenience :)

@victoredvardsson
Copy link
Contributor Author

Hello! Ok thanks for the quick reply, then we will hold off until that is fixed 👍

@buixor
Copy link
Contributor

buixor commented Nov 29, 2024

After investigation, the issue is the following:

  • With version 1.6.4 we introduced the ability to load several appsec-configs (ie. crowdsecurity/appsec-default and crowdsecurity/generic-rules)
  • However, Coraza forbids two rules with the same ID. We are thus deduplicating rules at load time. This is because of the way appsec-configs are made, the likelihood of two collections containing the same rule is quite high.

What went wrong:

  • We assumed that all SecRules have IDs
  • Because it's not the case (cf. the chain rules posted by @victoredvardsson ), we end up messing the rules, leading to unpredictable behaviour, as rules are deduplicated, but it might "just" delete a rule in the middle of a chain.

The fix:

  • Do not try to deduplicate native SecRules. We should only keep this behavior for "our" rules (because we know they always have individual IDs).

buixor added a commit that referenced this issue Nov 29, 2024
buixor added a commit that referenced this issue Nov 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants