Enhance WAF detection module with updated signatures and payloads #1282
Enhance WAF detection module with updated signatures and payloads #1282Aarush289 wants to merge 14 commits intoOWASP:masterfrom
Conversation
create pr
Document all modules ( fix OWASP#1269 ) (OWASP#1270)
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
Summary by CodeRabbit
WalkthroughExpanded the WAF detection YAML: replaced commented payloads with a comprehensive explicit payload list, added iterative_response_match threat-actor blocks, and introduced many new vendor detector entries with broader header/content/status patterns and logs. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nettacker/modules/scan/waf.yaml (1)
1-2250:⚠️ Potential issue | 🔴 Critical@Aarush289 — Unsigned commits block merge
All commits in pull requests to this repository must be GPG-signed. This PR will fail pre-merge checks if any commits are unsigned. Please sign all commits before requesting merge.
To sign commits: configure
git config --global commit.gpgsign truewith your GPG key, then amend/rebase to sign existing commits (git rebase --exec 'git commit --amend --no-edit -S' -i <base_commit>).Based on learnings from
securestep9: all commits in pull requests to OWASP/Nettacker must be GPG signed, and this is an explicit pre-merge requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` around lines 1 - 2250, The PR's commits are unsigned and must be GPG-signed before merging; configure Git to sign commits (set commit.gpgsign true and your signing key) and then sign the existing commits in this branch by amending/rebasing (interactive rebase from the base commit and reapply commits with signed amends) so all commits in this pull request are GPG-signed; once done, push the updated branch (force-push if you rebased) and update the PR.
🧹 Nitpick comments (1)
nettacker/modules/scan/waf.yaml (1)
382-389:condition_type: andwith a single condition across several new detector blocksThe following new blocks use
condition_type: andwith only one condition, which is functionally identical toorbut semantically inconsistent with every other single-condition block in the file (all of which useor):
- Azure Application Gateway (lines 382–389): single
contentcondition- FortiGuard (lines 890–897): single
contentcondition- NSFocus (lines 1301–1309): single
headers.Servercondition- Variti (lines 1905–1913): single
headers.Servercondition- Vercel WAF (lines 1922–1929): single
contentconditionConsider normalizing these to
condition_type: orfor consistency.Also applies to: 874-897, 1301-1309, 1905-1913, 1922-1929
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` around lines 382 - 389, Multiple new detector blocks in waf.yaml (Azure Application Gateway, FortiGuard, NSFocus, Variti, Vercel WAF) use condition_type: and while only defining a single condition; change each block's condition_type value from "and" to "or" to match the rest of the file and preserve semantic consistency—specifically update the Azure Application Gateway block (the response -> conditions -> content regex), the FortiGuard block (response -> conditions -> content), the NSFocus block (response -> conditions -> headers.Server), the Variti block (response -> conditions -> headers.Server), and the Vercel WAF block (response -> conditions -> content) to use condition_type: or.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 926-934: The current Cloud Armor signature "Google Cloud App Armor
(Google Cloud)" uses only the Via header regex "1\.1 google" (in the
conditions.headers.Via rule) which causes false positives for any Google CDN/GFE
traffic; update the signature to require a real block indicator by adding a
status_code condition (e.g., 403) and a Cloud Armor-specific response
body/header pattern (for example a Cloud Armor block page string or header)
alongside the Via check, or replace the Via-only rule with the more precise
fingerprint used by wafw00f for Cloud Armor to avoid matching all GFE responses.
- Around line 793-828: The "Envoy (EnvoyProxy)" detector in the waf.yaml uses
generic Envoy headers (e.g., Server: envoy and multiple x-envoy-* patterns)
which will flag any Envoy sidecar (Istio/App Mesh) as a WAF; narrow the
fingerprint in the "Envoy (EnvoyProxy)" response block to only match definitive
blocking indicators instead of general observability headers — remove or
de-prioritize matches for Server: envoy and headers like
x-envoy-upstream-service-time, x-envoy-internal, x-envoy-downstream-*, etc., and
replace them with checks for known blocking response markers (specific
error/body patterns or explicit x-envoy-reject/x-envoy-block headers or status
codes/body text unique to Envoy WAF deployments) so the detector only logs
"Envoy (EnvoyProxy) Found" when those stronger blocking signals are present.
- Around line 79-84: The YAML double-quoted payloads currently use `\\\"` which
parses to a backslash+quote; replace each `\\\"` with a single escaped quote
`\"` (or convert those scalars to single-quoted strings containing " directly)
so the parsed payloads contain a literal double-quote; specifically update the
payload entries like "<script>alert(\\\"NETTACKER\\\");</script>",
"\\\"><script>alert(\\\"XSS\\\");</script>" and the XXE entries using
"\\\"file:///etc/passwd\\\"" to use `\"` (or single-quoted scalars) so the
runtime strings become "<script>alert("NETTACKER");</script>",
"\"><script>alert("XSS");</script>" and <!ENTITY xxe SYSTEM
"file:///etc/passwd"> respectively.
- Around line 337-338: The regex for the Armor Defense rule contains a
duplicated alternative ("please create an armor support ticket") which is
redundant; update the regex value under the Armor Defense entry (the key named
regex in the shown block) to remove the duplicate alternative so each
alternation is unique (e.g., keep a single "please create an armor support
ticket" in the alternation list or dedupe programmatically).
- Around line 1506-1516: The detection rule "Reflected Networks (Reflected
Networks)" is producing false positives because the content regex includes the
generic alternative "<title>Forbidden</title>"; remove that third alternative
(or replace it with a Reflected Networks-specific marker) from the content regex
so the rule only matches the precise base64 PNG string and/or "content=\"Request
is denied\"", and keep the status_code 403 condition and condition_type: and in
place so the rule only fires for responses that truly match Reflected Networks
signatures.
- Around line 1660-1692: The Shieldon Firewall rules ("Shieldon Firewall
(Shieldon.io) condition1", "condition2", "condition3", "condition4") have their
log key misindented as a sibling of response:, so the framework never reads it;
move each log key to be a child of the response block (same indentation level as
conditions) for all four rules so the log entry is nested inside response and
will be emitted by the detection framework.
---
Outside diff comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 1-2250: The PR's commits are unsigned and must be GPG-signed
before merging; configure Git to sign commits (set commit.gpgsign true and your
signing key) and then sign the existing commits in this branch by
amending/rebasing (interactive rebase from the base commit and reapply commits
with signed amends) so all commits in this pull request are GPG-signed; once
done, push the updated branch (force-push if you rebased) and update the PR.
---
Nitpick comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 382-389: Multiple new detector blocks in waf.yaml (Azure
Application Gateway, FortiGuard, NSFocus, Variti, Vercel WAF) use
condition_type: and while only defining a single condition; change each block's
condition_type value from "and" to "or" to match the rest of the file and
preserve semantic consistency—specifically update the Azure Application Gateway
block (the response -> conditions -> content regex), the FortiGuard block
(response -> conditions -> content), the NSFocus block (response -> conditions
-> headers.Server), the Variti block (response -> conditions -> headers.Server),
and the Vercel WAF block (response -> conditions -> content) to use
condition_type: or.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
nettacker/modules/scan/waf.yaml (1)
1-12:⚠️ Potential issue | 🔴 Critical
⚠️ Unsigned commits will block merge —@Aarush289, please sign all commitsThe OWASP/Nettacker project requires signed commits as part of its pre-merge checks. This PR will fail pre-merge checks if any commits are unsigned.
To sign commits retroactively:
# Sign the last N commits (replace N with the number of commits in this PR) git rebase HEAD~N --exec 'git commit --amend --no-edit -S' git push --force-with-leaseTo configure Git to sign all future commits automatically:
git config --global commit.gpgsign trueBased on learnings: the OWASP/Nettacker repository requires signed commits and the PR fails pre-merge checks if any commits are unsigned.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` around lines 1 - 12, The commits for this change (module name waf_scan in nettacker/modules/scan/waf.yaml) must be GPG-signed to pass pre-merge checks; please sign any unsigned commits in this PR by rebasing and amending those commits with GPG signing enabled, then force-push the branch, and enable automatic commit signing in your local Git configuration so future commits are signed by default.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 1761-1775: The X-Request-ID header check in the ThreatX (A10
Networks) detector is non-discriminatory (regex: .*) and should be removed or
replaced; edit the ThreatX (A10 Networks) detector block to delete the headers
-> X-Request-ID condition (or swap it for a real ThreatX-specific header if you
have one) while keeping the existing status_code and content conditions (the
content regex '^Forbidden - ID\: ([a-fA-F0-9]{32})$' and status_code '403') and
leave condition_type: and intact.
- Around line 1877-1884: The current rule "Vercel WAF (Vercel)" in waf.yaml uses
conditions.content.regex containing the generic alternative "/vercel/security/"
which causes false positives; replace that alternative with a stricter regex
that only matches the path when it appears in URL contexts (e.g.
anchor/link/href or full URL) such as a pattern that looks for href or link
attributes or full URLs (for example match
(<a[^>]+href=['"]\/vercel\/security\/|href=['"]https?:\/\/[^'"]+\/vercel\/security\/)
) so the detector still catches real WAF challenge pages but not ordinary pages
that merely mention the path; update the regex in conditions.content.regex for
the "Vercel WAF (Vercel)" rule accordingly.
- Around line 370-381: The Azion Edge Firewall detector currently triggers on
generic headers (x-azion-edge-pop / x-azion-request-id) with condition_type: or
which yields false positives for any Azion CDN; update the "Azion Edge Firewall
(Azion)" entry to stop using those routing headers and instead require a
WAF-specific blocking signal such as a non-2xx response plus a block-page body
pattern: remove or stop relying on x-azion-edge-pop and x-azion-request-id,
change the condition_type to require both a status code regex matching 4xx/5xx
and a body regex that matches Azion WAF block page content (e.g., known
block-page text/snippets), and keep CDN detection separate (the existing
"AzionCDN (AzionCDN)" detector using Server: Azion[-_]CDN should remain as-is).
---
Outside diff comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 1-12: The commits for this change (module name waf_scan in
nettacker/modules/scan/waf.yaml) must be GPG-signed to pass pre-merge checks;
please sign any unsigned commits in this PR by rebasing and amending those
commits with GPG signing enabled, then force-push the branch, and enable
automatic commit signing in your local Git configuration so future commits are
signed by default.
---
Duplicate comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 79-84: The YAML strings in waf.yaml contain literal backslashes
before quotes (e.g. '<script>alert(\"NETTACKER\");</script>',
'\"><script>alert(\"XSS\");</script>') because `\"` inside single-quoted YAML is
not an escape; remove the backslashes so the payloads contain real double quotes
(e.g. '<script>alert("NETTACKER");</script>' and
'"?><script>alert("XSS");</script>') and ensure the same replacement is applied
to the other occurrences noted in the comment (lines with
'<script>alert(\"test\");</script>' and the two entries referenced at lines
123–124) so the parsed values are correct.
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
nettacker/modules/scan/waf.yaml (1)
370-377:condition_type: andwith a single condition is misleading — useorfor consistencyFour new detector entries use
condition_type: andwith only one entry underconditions:, which is logically identical toor. Every other single-condition detector in this file usesor. Affected new blocks:
Block Lines Azure Application Gateway (Microsoft) 370–377 FortiGuard (Fortinet) 842–849 NSFocus (NSFocus Global Inc.) 1244–1252 Variti (Variti) 1844–1852 ♻️ Proposed fix (apply to all four blocks)
- condition_type: and + condition_type: orAlso applies to: 842-849, 1244-1252, 1844-1852
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` around lines 370 - 377, The detector entries for "Azure Application Gateway (Microsoft)", "FortiGuard (Fortinet)", "NSFocus (NSFocus Global Inc.)", and "Variti (Variti)" use condition_type: and while each defines only a single condition; change condition_type: and to condition_type: or in each of these detector blocks (look for the response -> condition_type field under those detector names) so the logic matches the rest of the file and single-condition detectors.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 842-849: The FortiGuard (Fortinet) detector's content regex
("FortiGuard Intrusion Prevention|//globalurl.fortinet.net") uses unescaped dots
which match any character; update the regex in the "FortiGuard (Fortinet)" block
(the content -> regex entry) to escape the dots in the hostname (use `\.`, i.e.
`//globalurl\.fortinet\.net`) so it matches the literal domain and aligns with
the adjacent FortiGate detectors' pattern.
- Line 76: This PR's commits are unsigned and must be GPG-signed to meet the
repository policy; configure your local git to use your GPG key and enable
commit signing, then retroactively sign all commits on this branch (e.g.,
rebase/amend each commit to add the signature) and force-push the branch with
lease so the PR contains only signed commits; ensure you target the branch
containing the changes to waf.yaml and verify signatures locally before pushing.
---
Duplicate comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 79-84: The YAML payload entries in waf.yaml still contain literal
backslashes because single-quoted scalars do not use backslash escapes (e.g.,
the entries containing '<script>alert(\"NETTACKER\");</script>' etc.); update
those payload strings so they contain the actual quotes without
backslashes—replace occurrences of \" inside single-quoted strings with plain "
(or switch to double-quoted strings and unescape them properly) for the listed
XSS payloads (the script/img entries shown in the diff) and apply the same fix
to the other similar entries referenced (the duplicates around the other block).
---
Nitpick comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 370-377: The detector entries for "Azure Application Gateway
(Microsoft)", "FortiGuard (Fortinet)", "NSFocus (NSFocus Global Inc.)", and
"Variti (Variti)" use condition_type: and while each defines only a single
condition; change condition_type: and to condition_type: or in each of these
detector blocks (look for the response -> condition_type field under those
detector names) so the logic matches the rest of the file and single-condition
detectors.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
nettacker/modules/scan/waf.yaml (1)
370-377: Four new single-condition detectors usecondition_type: and— should beorfor consistencyAll single-condition detector blocks elsewhere in this file use
condition_type: or. The following new entries useandwith only one condition, which is functionally equivalent but breaks the established convention:
Detector Line Single condition Azure Application Gateway 372 content:FortiGuard 844 content:NSFocus 1246 headers.ServerVariti 1846 headers.Server♻️ Proposed fix (representative example — apply to all four)
Azure Application Gateway (Microsoft): response: - condition_type: and + condition_type: or conditions: content: regex: ...Also applies to: 842-849, 1244-1252, 1844-1852
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` around lines 370 - 377, The four new single-condition detectors (Azure Application Gateway, FortiGuard, NSFocus, Variti) incorrectly use response.condition_type: and despite having only one condition; update each detector's response block to use condition_type: or for consistency with the rest of the file (specifically change the response.condition_type for the Azure Application Gateway detector that contains the content regex, the FortiGuard detector with its content condition, the NSFocus detector that checks headers.Server, and the Variti detector that checks headers.Server).
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 425-428: The Set-Cookie regex check (headers -> Set-Cookie:
TS[a-fA-F0-9]{{8}}=.+|TS[a-fA-F0-9]{{6}}=.+) is currently combined with the
content check under an AND, causing false negatives for BIG-IP AppSec; change
the rule so the TS cookie check is an OR-style condition by moving this
headers/Set-Cookie check into its own condition2 block or by adding it as a
separate top-level condition with condition_type: or (or create a separate
"BIG-IP AppSec Manager (F5 Networks) condition2" block mirroring the existing
BIG-IP AP Manager split) so detection succeeds when either the content signature
OR the TS cookie is present.
---
Duplicate comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 79-84: The YAML uses single-quoted scalars but contains
backslash-escaped quotes (e.g. '<script>alert(\"NETTACKER\");</script>' and
'\"><script>alert(\"XSS\");</script>'), and in single quotes `\"` is literal (a
backslash plus quote); fix by removing the backslashes and either (a) keep
single quotes and use real double-quote characters inside (e.g.
'<script>alert("NETTACKER");</script>') or (b) switch the outer quotes to
double-quoted YAML and unescape as needed (e.g.
"<script>alert(\"NETTACKER\");</script>"); apply the same change to all similar
payloads such as the alert/test/XSS entries and the other affected entries
mentioned in the comment.
- Line 1613: The YAML key/value "condition_type: or " in the Shieldon condition2
entry has a trailing space; locate the Shieldon condition2 block (look for
condition2 / condition_type) and remove the trailing space so the value becomes
"or" (i.e., change condition_type: or to condition_type: or) to ensure clean,
parser-safe YAML.
- Line 76: The PR's commits are not GPG-signed (pre-merge checks failing for PR
`#1282`); fix by configuring your git signing key and enabling commit signing
locally (set user.signingkey and commit.gpgsign), then retroactively sign the
branch's commits by rebasing and amending each commit with GPG signing (use a
rebase that runs git commit --amend -S on each commit), and finally force-push
the branch with lease so the signed commits replace the unsigned ones; after
pushing, verify the commits on the PR are marked verified.
---
Nitpick comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 370-377: The four new single-condition detectors (Azure
Application Gateway, FortiGuard, NSFocus, Variti) incorrectly use
response.condition_type: and despite having only one condition; update each
detector's response block to use condition_type: or for consistency with the
rest of the file (specifically change the response.condition_type for the Azure
Application Gateway detector that contains the content regex, the FortiGuard
detector with its content condition, the NSFocus detector that checks
headers.Server, and the Variti detector that checks headers.Server).
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
nettacker/modules/scan/waf.yaml (2)
419-419: Trailing whitespace oncondition_type: orLines 419 and 1613 both have a trailing space after
or. Most YAML parsers are lenient, but it's worth cleaning up for consistency.🧹 Proposed fix
- condition_type: or + condition_type: or(Apply to both occurrences.)
Also applies to: 1613-1613
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` at line 419, Remove the trailing space after the value for the YAML key condition_type where it is set to "or" in nettacker/modules/scan/waf.yaml (the two occurrences where condition_type: or has an extra space); update those entries to use "condition_type: or" (no trailing whitespace) to keep the file consistent and avoid potential parser/linters issues.
625-625:cloudflare[-_]nginxalternative is redundantThe broader alternative
cloudflarealready matches any Server value containingcloudflare[-_]nginx. The first alternative can be dropped.🧹 Proposed fix
- regex: cloudflare[-_]nginx|cloudflare + regex: cloudflare🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@nettacker/modules/scan/waf.yaml` at line 625, The regex "cloudflare[-_]nginx|cloudflare" is redundant because "cloudflare" already matches the longer form; update the rule in nettacker/modules/scan/waf.yaml that defines regex to use just "cloudflare" (remove the "cloudflare[-_]nginx" alternative) so the pattern is simplified and equivalent.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 1015-1029: The Kemp LoadMaster fingerprint currently requires
three conditions (headers X-ServedBy, status_code 403, and content "<title>403
Forbidden</title>") combined with condition_type: and, which causes false
negatives; remove the content condition block (the content: regex: <title>403
Forbidden</title>) so the fingerprint relies on the specific X-ServedBy header
and the 403 status_code only (keep condition_type: and and the
headers/status_code blocks as-is, and leave the log message unchanged).
---
Duplicate comments:
In `@nettacker/modules/scan/waf.yaml`:
- Around line 79-84: Replace literal backslash-escaped quotes inside
single-quoted YAML scalars with plain double-quote characters so the parsed
payloads do not contain a stray backslash; e.g., change occurrences like
'<script>alert(\"NETTACKER\");</script>' and
'\"><script>alert(\"XSS\");</script>' to '<script>alert("NETTACKER");</script>'
and '\"><script>alert("XSS");</script>' (and similarly for the other affected
entries referenced in the comment) in nettacker/modules/scan/waf.yaml.
- Line 76: All commits in this PR are not GPG-signed; sign them and re-push so
the repo policy passes: configure your git signing key and enable
commit.gpgsign, then retroactively sign every commit on this branch by rebasing
with an amend-and-sign exec (the suggested rebase with git rebase --exec 'git
commit --amend --no-edit -S' origin/master) and push the branch with
--force-with-lease; after that verify commit signatures for PR `#1282` using the
provided GH API check to ensure all commits are marked verified.
---
Nitpick comments:
In `@nettacker/modules/scan/waf.yaml`:
- Line 419: Remove the trailing space after the value for the YAML key
condition_type where it is set to "or" in nettacker/modules/scan/waf.yaml (the
two occurrences where condition_type: or has an extra space); update those
entries to use "condition_type: or" (no trailing whitespace) to keep the file
consistent and avoid potential parser/linters issues.
- Line 625: The regex "cloudflare[-_]nginx|cloudflare" is redundant because
"cloudflare" already matches the longer form; update the rule in
nettacker/modules/scan/waf.yaml that defines regex to use just "cloudflare"
(remove the "cloudflare[-_]nginx" alternative) so the pattern is simplified and
equivalent.
Proposed change
This PR enhances the existing WAF detection module, which was originally implemented in 2018 and has not been updated to reflect the current WAF ecosystem.
The following improvements were made:
Added detection signatures for newer WAF services that were missing from the original module.
Updated and aligned fingerprints based on the latest implementations in:
Wafw00f (latest version)
Nuclei WAF detection templates
Expanded the set of payloads used in GET parameter testing.
Type of change
Checklist
make pre-commit, it didn't generate any changesmake test, all tests passed locally