Add active analyzers#7546
Conversation
WalkthroughAdds shared fuzz-analyzer request utilities, five baseline-aware analyzers, four reflection-oriented analyzers, analyzer playground routes, and extensive unit, integration, and end-to-end coverage across query, path, header, body, and cookie inputs. ChangesAnalyzer execution and detection
Estimated code review effort: 5 (Critical) | ~120 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 8
🧹 Nitpick comments (2)
pkg/fuzz/analyzers/e2e/baseline_guard_test.go (1)
1-7: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd SSTI to the baseline-guard test matrix.
The PR introduces five baseline-aware analyzers, but this suite covers only SQLi, LFI, SSRF, and CMDi. Add an SSTI case proving a baseline that already satisfies its detection signal is not reported.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/fuzz/analyzers/e2e/baseline_guard_test.go` around lines 1 - 7, Add an SSTI baseline-guard case to the test matrix in baseline_guard_test.go, using a server response that unconditionally contains the SSTI analyzer’s detection signature, including for the original value. Assert the SSTI analyzer reports no vulnerability, matching the existing SQLi, LFI, SSRF, and CMDi baseline tests.pkg/fuzz/analyzers/e2e/pipeline_test.go (1)
1-11: 📐 Maintainability & Code Quality | 🔵 Trivial | 🏗️ Heavy liftExercise body, cookie, and header components through the real pipeline.
The request fixture is hard-coded to GET with no body or headers, and the matrix covers only query/path. Consequently,
/accountand/postsare unused and component discovery/rebuilding for body, cookie, and header inputs remains untested at the pipeline layer. Generalize the fixture and add those cases before claiming coverage across all positions.Also applies to: 217-224, 268-286
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pkg/fuzz/analyzers/e2e/pipeline_test.go` around lines 1 - 11, Generalize the real pipeline request fixture and test matrix in the end-to-end analyzer tests so request method, body, cookies, and headers can be configured per case instead of remaining GET-only with no inputs. Add cases exercising body, cookie, and header mutations against the existing /account and /posts vulnerable routes, and ensure each case still runs through Request.ExecuteWithResults and validates the analyzer finding.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@pkg/fuzz/analyzers/analyzers.go`:
- Around line 107-120: The header restoration logic in the request rebuild flow
must merge missing cookie pairs into an existing Cookie header instead of
skipping it when rebuilt.Header.Values(k) is non-empty. Update the handling
around gr.Request.Header and rebuilt.Header so post-parse session cookies are
preserved, while retaining the existing skip behavior for actively fuzzed
headers and component-managed non-cookie headers.
In `@pkg/fuzz/analyzers/cors/analyzer.go`:
- Around line 80-83: Update the targetHost assignment in the request URL guard
to call Hostname() on gr.Request.URL rather than gr.Request, preserving the
existing nil checks before accessing the URL.
In `@pkg/fuzz/analyzers/crlf/analyzer.go`:
- Around line 68-71: Update the Set-Cookie matching logic in the analyzer to
require an exact crlf=<value> cookie pair instead of matching the value within
any cookie. Add a negative test in pkg/fuzz/analyzers/crlf/analyzer_test.go
covering an unrelated cookie such as session=deadbeef, while preserving the
existing positive match behavior.
In `@pkg/fuzz/analyzers/hostheader/analyzer_test.go`:
- Around line 30-44: The host-header analyzer’s substring matching incorrectly
accepts canary prefixes within attacker-controlled hostnames. Add a negative
test for `abc123.hostcanary.example.attacker.example`, then update the
analyzer’s hostname detection logic to parse URL hosts and require an exact
hostname match rather than accepting a host prefix.
In `@pkg/fuzz/analyzers/hostheader/analyzer.go`:
- Around line 63-69: Update the host-matching logic in the analyzer’s
canary-check function to require the canaryHost to be an exact hostname, not a
prefix of a longer domain. Accept matches only when the character immediately
after the canary is a valid host delimiter such as a port, path, query,
fragment, or URL boundary, while preserving support for absolute,
scheme-relative, and userinfo-prefixed URLs.
- Around line 84-95: Update the probe construction in the loop over probes so
the Forwarded header uses RFC 7239 syntax, setting its value to host=<canary>
rather than the bare canary. Keep the existing direct Host assignment and other
override-header behavior unchanged.
In `@pkg/fuzz/analyzers/redirect/analyzer.go`:
- Around line 92-96: Update the redirect-follow error handling so a non-nil
response is preserved and processed even when HttpClient.Do returns an error;
only skip the probe when no response exists. Apply this in
pkg/fuzz/analyzers/redirect/analyzer.go lines 92-96 and
pkg/fuzz/analyzers/hostheader/analyzer.go lines 98-100, ensuring the retained
response continues through the existing Location/final-URL analysis.
In `@pkg/fuzz/analyzers/sqli/analyzer.go`:
- Around line 106-108: Restore each component’s internal state after resetting
its value by adding the ignored-result Rebuild call immediately after SetValue
in the defer cleanup blocks for pkg/fuzz/analyzers/sqli/analyzer.go lines
106-108, pkg/fuzz/analyzers/ssrf/analyzer.go lines 97-99, and
pkg/fuzz/analyzers/lfi/analyzer.go lines 93-95.
---
Nitpick comments:
In `@pkg/fuzz/analyzers/e2e/baseline_guard_test.go`:
- Around line 1-7: Add an SSTI baseline-guard case to the test matrix in
baseline_guard_test.go, using a server response that unconditionally contains
the SSTI analyzer’s detection signature, including for the original value.
Assert the SSTI analyzer reports no vulnerability, matching the existing SQLi,
LFI, SSRF, and CMDi baseline tests.
In `@pkg/fuzz/analyzers/e2e/pipeline_test.go`:
- Around line 1-11: Generalize the real pipeline request fixture and test matrix
in the end-to-end analyzer tests so request method, body, cookies, and headers
can be configured per case instead of remaining GET-only with no inputs. Add
cases exercising body, cookie, and header mutations against the existing
/account and /posts vulnerable routes, and ensure each case still runs through
Request.ExecuteWithResults and validates the analyzer finding.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e5c8b818-f523-4f86-8c2e-f12d9eb3e88a
⛔ Files ignored due to path filters (14)
internal/tests/integration/testdata/fuzz/analyzer-body-sqli.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-cmdi.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-cookie-ssti.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-cors.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-crlf.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-header-sqli.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-host-header.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-lfi.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-open-redirect.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-path-sqli.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-sqli.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-ssrf.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/analyzer-ssti.yamlis excluded by!**/*.yamlinternal/tests/integration/testdata/fuzz/testData/analyzer-positions.proxify.yamlis excluded by!**/*.yaml
📒 Files selected for processing (26)
internal/fuzzplayground/server.gointernal/tests/integration/fuzz_test.gopkg/fuzz/analyzers/analyzers.gopkg/fuzz/analyzers/cmdi/analyzer.gopkg/fuzz/analyzers/cmdi/analyzer_test.gopkg/fuzz/analyzers/cors/analyzer.gopkg/fuzz/analyzers/cors/analyzer_test.gopkg/fuzz/analyzers/crlf/analyzer.gopkg/fuzz/analyzers/crlf/analyzer_test.gopkg/fuzz/analyzers/e2e/baseline_guard_test.gopkg/fuzz/analyzers/e2e/e2e_test.gopkg/fuzz/analyzers/e2e/pipeline_test.gopkg/fuzz/analyzers/hostheader/analyzer.gopkg/fuzz/analyzers/hostheader/analyzer_test.gopkg/fuzz/analyzers/lfi/analyzer.gopkg/fuzz/analyzers/lfi/analyzer_test.gopkg/fuzz/analyzers/redirect/analyzer.gopkg/fuzz/analyzers/redirect/analyzer_test.gopkg/fuzz/analyzers/sqli/analyzer.gopkg/fuzz/analyzers/sqli/analyzer_test.gopkg/fuzz/analyzers/ssrf/analyzer.gopkg/fuzz/analyzers/ssrf/analyzer_test.gopkg/fuzz/analyzers/ssti/analyzer.gopkg/fuzz/analyzers/ssti/analyzer_test.gopkg/fuzz/analyzers/time/analyzer.gopkg/fuzz/analyzers/xss/analyzer.go
| if gr.Request != nil { | ||
| for k, vs := range gr.Request.Header { | ||
| // don't clobber the header we are actively fuzzing | ||
| if gr.Component.Name() == "header" && k == gr.Key { | ||
| continue | ||
| } | ||
| // don't clobber headers the component itself manages on the rebuilt | ||
| // request (e.g. Cookie for the cookie component, Content-Type/Length | ||
| // for the body component); only restore headers that Rebuild dropped | ||
| // (most importantly post-parse auth headers). | ||
| if len(rebuilt.Header.Values(k)) > 0 { | ||
| continue | ||
| } | ||
| rebuilt.Header[k] = vs |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Inspect cookie rebuilding and post-parse authentication injection.
fd -i 'cookie.go|auth.*\.go' . -x sh -c 'echo "===== $1"; ast-grep outline "$1" --items all' sh {}
rg -n -C4 --type=go 'Header\.(Set|Add)\("Cookie"|AddCookie\(|SetCookie\(|Component.*cookie|func .*Rebuild\(' .Repository: projectdiscovery/nuclei
Length of output: 17845
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== pkg/fuzz/analyzers/analyzers.go ==="
sed -n '90,130p' pkg/fuzz/analyzers/analyzers.go
echo
echo "=== pkg/fuzz/component/cookie.go ==="
sed -n '1,130p' pkg/fuzz/component/cookie.go
echo
echo "=== pkg/authprovider/authx/cookies_auth.go ==="
sed -n '1,120p' pkg/authprovider/authx/cookies_auth.go
echo
echo "=== pkg/fuzz/analyzers/time/time_delay_test.go (relevant cookie/header tests) ==="
sed -n '500,620p' pkg/fuzz/analyzers/time/time_delay_test.goRepository: projectdiscovery/nuclei
Length of output: 9066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "=== pkg/fuzz/analyzers/analyzers.go ==="
sed -n "90,130p" pkg/fuzz/analyzers/analyzers.go
echo
echo "=== pkg/fuzz/component/cookie.go ==="
sed -n "1,130p" pkg/fuzz/component/cookie.go
echo
echo "=== pkg/authprovider/authx/cookies_auth.go ==="
sed -n "1,120p" pkg/authprovider/authx/cookies_auth.go
echo
echo "=== pkg/fuzz/analyzers/time/time_delay_test.go (relevant cookie/header tests) ==="
sed -n "500,620p" pkg/fuzz/analyzers/time/time_delay_test.goRepository: projectdiscovery/nuclei
Length of output: 9066
🏁 Script executed:
#!/bin/bash
set -euo pipefail
rg -n -C3 'SetValueAndRebuild\(' pkg/fuzz/analyzersRepository: projectdiscovery/nuclei
Length of output: 4248
Preserve post-parse session cookies when rebuilding cookie requests.
len(rebuilt.Header.Values(k)) > 0 skips restoring the entire Cookie header whenever one already exists, so cookies injected after parse can be dropped and analyzers may probe unauthenticated. Merge missing cookie pairs back in instead of treating any existing Cookie header as complete.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/analyzers.go` around lines 107 - 120, The header
restoration logic in the request rebuild flow must merge missing cookie pairs
into an existing Cookie header instead of skipping it when
rebuilt.Header.Values(k) is non-empty. Update the handling around
gr.Request.Header and rebuilt.Header so post-parse session cookies are
preserved, while retaining the existing skip behavior for actively fuzzed
headers and component-managed non-cookie headers.
| targetHost := "" | ||
| if gr.Request != nil && gr.Request.URL != nil { | ||
| targetHost = gr.Request.Hostname() | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🔴 Critical | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the target file around the reported lines
sed -n '1,140p' pkg/fuzz/analyzers/cors/analyzer.go
# Locate the type of gr.Request and any Hostname() usage in the repo
rg -n "Hostname\(\)|type .*Request|RequestHost|gr\.Request" pkg/fuzz/analyzers -SRepository: projectdiscovery/nuclei
Length of output: 4468
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file structure first, then inspect the relevant slices
ast-grep outline pkg/fuzz/analyzers/cors/analyzer.go --view expanded || true
sed -n '1,140p' pkg/fuzz/analyzers/cors/analyzer.go
# Search for the request type definition and Hostname call sites
rg -n "type .*Request|Hostname\(\)|gr\.Request" pkg/fuzz -SRepository: projectdiscovery/nuclei
Length of output: 5030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
ast-grep outline pkg/fuzz/analyzers/cors/analyzer.go --view expanded || true
sed -n '1,140p' pkg/fuzz/analyzers/cors/analyzer.go
rg -n "type .*Request|Hostname\(\)|gr\.Request" pkg/fuzz -SRepository: projectdiscovery/nuclei
Length of output: 5030
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Inspect the GeneratedRequest type and the exact type of Request
sed -n '1,180p' pkg/fuzz/execute.go
# Check whether any custom Request wrapper defines Hostname()
rg -n "type .*Request struct|func \(.*\) Hostname\(" pkg/fuzz -SRepository: projectdiscovery/nuclei
Length of output: 6775
🌐 Web query:
projectdiscovery retryablehttp-go Request Hostname method URL field
💡 Result:
The projectdiscovery/retryablehttp-go library defines a Request struct that wraps the standard net/http Request [1][2]. Within this struct, there are two primary ways to access and manage URL and Hostname information: 1. Embedded http.Request: The Request struct embeds *http.Request [1][2]. This means all fields present in the standard library's http.Request (such as Host, URL, and Method) are directly available and accessible on the retryablehttp.Request object [1][2]. 2. URL Field: The struct explicitly includes a field *urlutil.URL [1][2]. This custom URL type is used by the library to handle URL normalization and parsing issues that the standard Go url.URL might struggle with [3]. When working with these fields, note the following: - Method and Host are accessed via the embedded *http.Request field (e.g., req.Method, req.Host) [1][2]. - The library recommends against manually updating the custom URL field (e.g., req.URL.Path = "...") directly because of internal parsing logic [3]. If modifications are necessary, users should use the Request.Update method to commit changes, particularly for query parameters [3].
Citations:
- 1: https://github.com/projectdiscovery/retryablehttp-go/blob/main/request.go
- 2: https://github.com/projectdiscovery/retryablehttp-go/blob/master/request.go
- 3: https://github.com/projectdiscovery/retryablehttp-go/blob/main/README.md
🌐 Web query:
site:github.com/projectdiscovery/retryablehttp-go "func (.*Request) Hostname" OR "Hostname()"
💡 Result:
There is no method with the signature func (.*Request) Hostname Hostname in the projectdiscovery/retryablehttp-go library [1][2][3]. The library, which is a wrapper around the standard net/http package, exposes a Request structure that includes an embedded *http.Request and a custom URL field of type *urlutil.URL [1][2]. Accessing or modifying the hostname for a request in this library is typically done by interacting directly with the Host field of the underlying http.Request or the URL field [4]. Based on the provided documentation and source code, no such method exists [1][2][5].
Citations:
- 1: https://github.com/projectdiscovery/retryablehttp-go/blob/main/request.go
- 2: https://github.com/projectdiscovery/retryablehttp-go/blob/master/request.go
- 3: https://github.com/projectdiscovery/retryablehttp-go/blob/main/methods.go
- 4: advised to add a feature retryablehttp-go#24
- 5: https://github.com/projectdiscovery/retryablehttp-go/blob/main/doc.go
🌐 Web query:
site:github.com/projectdiscovery/retryablehttp-go "Hostname()" "Request"
💡 Result:
Based on technical research, there is no standard programming function or HTTP request method named Hostname. The term is likely a misunderstanding or a specific internal reference within a niche library or codebase rather than a generally recognized standard. In the context of Go programming and common HTTP client libraries (such as the ProjectDiscovery retryablehttp library, which often appears in security-related technical research), Hostname is typically a property or field—not a function—used to identify the host component of a URL or request [1][2]. If you are encountering this in a specific codebase or log, it is likely one of the following: 1. A custom-defined function or helper method specific to that project. 2. A reference to the Host field of an HTTP request object (e.g., Request.URL.Host), which is standard across Go's net/http package and its wrappers [1][2]. To identify the exact usage, you should search the source code of the project where you encountered the term to see how it is defined and implemented.
Citations:
- 1: https://github.com/projectdiscovery/retryablehttp-go/blob/main/request.go
- 2: https://github.com/projectdiscovery/retryablehttp-go/blob/master/request.go
Call Hostname() on gr.Request.URL. retryablehttp.Request doesn't expose Hostname(), so this won't compile as written.
Proposed fix
if gr.Request != nil && gr.Request.URL != nil {
- targetHost = gr.Request.Hostname()
+ targetHost = gr.Request.URL.Hostname()
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| targetHost := "" | |
| if gr.Request != nil && gr.Request.URL != nil { | |
| targetHost = gr.Request.Hostname() | |
| } | |
| targetHost := "" | |
| if gr.Request != nil && gr.Request.URL != nil { | |
| targetHost = gr.Request.URL.Hostname() | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/cors/analyzer.go` around lines 80 - 83, Update the
targetHost assignment in the request URL guard to call Hostname() on
gr.Request.URL rather than gr.Request, preserving the existing nil checks before
accessing the URL.
| for _, c := range h.Values("Set-Cookie") { | ||
| if strings.Contains(c, value) { | ||
| return true | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Match the exact injected cookie to prevent false positives.
pkg/fuzz/analyzers/crlf/analyzer.go#L68-L71: require the cookie pair to equalcrlf=<value>, rather than searching every cookie for the token.pkg/fuzz/analyzers/crlf/analyzer_test.go#L45-L49: add a negative case such asSet-Cookie: session=deadbeef.
📍 Affects 2 files
pkg/fuzz/analyzers/crlf/analyzer.go#L68-L71(this comment)pkg/fuzz/analyzers/crlf/analyzer_test.go#L45-L49
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/crlf/analyzer.go` around lines 68 - 71, Update the
Set-Cookie matching logic in the analyzer to require an exact crlf=<value>
cookie pair instead of matching the value within any cookie. Add a negative test
in pkg/fuzz/analyzers/crlf/analyzer_test.go covering an unrelated cookie such as
session=deadbeef, while preserving the existing positive match behavior.
| { | ||
| name: "canary in absolute URL in body", | ||
| body: `<a href="https://abc123.hostcanary.example/reset">reset</a>`, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "canary in scheme-relative URL in body", | ||
| body: `<script src="//abc123.hostcanary.example/app.js"></script>`, | ||
| want: true, | ||
| }, | ||
| { | ||
| name: "canary after userinfo @ in body", | ||
| body: `redirect to http://user@abc123.hostcanary.example/`, | ||
| want: true, | ||
| }, |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Test and enforce exact hostname boundaries.
The current substring detector also matches https://abc123.hostcanary.example.attacker.example/. Add this as a negative case and parse URL hosts rather than accepting host prefixes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/hostheader/analyzer_test.go` around lines 30 - 44, The
host-header analyzer’s substring matching incorrectly accepts canary prefixes
within attacker-controlled hostnames. Add a negative test for
`abc123.hostcanary.example.attacker.example`, then update the analyzer’s
hostname detection logic to parse URL hosts and require an exact hostname match
rather than accepting a host prefix.
| // match the canary only when it appears as a URL host (scheme-relative or | ||
| // absolute), not as an arbitrary substring, to avoid accidental matches. | ||
| lc := strings.ToLower(body) | ||
| cl := strings.ToLower(canaryHost) | ||
| return strings.Contains(lc, "//"+cl) || | ||
| strings.Contains(lc, "://"+cl) || | ||
| strings.Contains(lc, "@"+cl) |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require an exact hostname match in response bodies.
https://<canary>.legitimate.example/ currently matches //<canary> even though the canary is only a hostname prefix. Parse candidate URLs or require a valid host delimiter immediately after the canary to avoid confirmed-finding false positives.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/hostheader/analyzer.go` around lines 63 - 69, Update the
host-matching logic in the analyzer’s canary-check function to require the
canaryHost to be an exact hostname, not a prefix of a longer domain. Accept
matches only when the character immediately after the canary is a valid host
delimiter such as a port, path, query, fragment, or URL boundary, while
preserving support for absolute, scheme-relative, and userinfo-prefixed URLs.
| // Probe each override header individually, plus a direct Host override. | ||
| probes := append([]string{"Host"}, overrideHeaders...) | ||
| for _, header := range probes { | ||
| rebuilt, err := analyzers.SetValueAndRebuild(gr, gr.OriginalValue) | ||
| if err != nil { | ||
| return false, "", err | ||
| } | ||
| if header == "Host" { | ||
| rebuilt.Host = canary | ||
| } else { | ||
| rebuilt.Header.Set(header, canary) | ||
| } |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
set -euo pipefail
printf '--- file outline ---\n'
ast-grep outline pkg/fuzz/analyzers/hostheader/analyzer.go --view expanded || true
printf '\n--- relevant slice ---\n'
nl -ba pkg/fuzz/analyzers/hostheader/analyzer.go | sed -n '1,180p'
printf '\n--- search Forwarded references ---\n'
rg -n '"Forwarded"|Forwarded|host=' pkg/fuzz . -g '!**/vendor/**'Repository: projectdiscovery/nuclei
Length of output: 252
🏁 Script executed:
set -euo pipefail
nl -ba pkg/fuzz/analyzers/hostheader/analyzer.go | sed -n '1,180p'
printf '\n--- search Forwarded references ---\n'
rg -n 'Forwarded|host=' pkg/fuzz . -g '!**/vendor/**'Repository: projectdiscovery/nuclei
Length of output: 201
🌐 Web query:
RFC 7239 forwarded header host parameter syntax host= forwarded header value parsing
💡 Result:
The Forwarded HTTP header, as specified in RFC 7239, uses the "host" parameter to communicate the original value of the "Host" request header field as it was received by a proxy [1][2]. Syntax Requirements The syntax for the "host" parameter value in the Forwarded header must follow these rules: 1. General Parameter Syntax: Like all parameters in the Forwarded header, the "host" parameter must be represented as a key-value pair (e.g., host=example.com) [3][4]. The value may be a token or a quoted-string [5]. 2. Unescaping: If the value is provided as a quoted-string, any escape sequences must be unescaped prior to validation [1][2]. 3. Conformance: After any necessary unescaping, the resulting host value must conform strictly to the Host ABNF syntax defined in Section 5.4 of RFC 7230 [1][2]. Parsing Guidelines When parsing the Forwarded header, implementers must handle the following: - Case Insensitivity: Parameter names are case-insensitive [3][6]. - Structure: The header can contain multiple elements, which are separated by commas [3]. Within each element, multiple parameters are separated by semicolons [4]. - Quoting and Escaping: Parsers must support both unquoted tokens and quoted strings, ensuring that characters escaped within quoted strings are processed correctly according to the RFC's requirements before validating the host format [5]. Because RFC 7239 requires the resulting host value to match the Host ABNF, it effectively mandates compliance with the syntax for host and optional port components (e.g., host:port or [ipv6]:port) as defined in the referenced HTTP/1.1 specifications [1][2].
Citations:
- 1: https://www.rfc-editor.org/info/rfc7239/
- 2: https://datatracker.ietf.org/doc/html/rfc7239.html
- 3: https://www.rfc-editor.org/rfc/rfc7239.txt
- 4: https://developer.mozilla.org/en-US/docs/Web/HTTP/Reference/Headers/Forwarded
- 5: https://falcon.readthedocs.io/en/4.2.0/_modules/falcon/forwarded.html
- 6: https://datatracker.ietf.org/doc/rfc7239/
🏁 Script executed:
set -euo pipefail
echo '--- target slice ---'
sed -n '70,110p' pkg/fuzz/analyzers/hostheader/analyzer.go
echo
echo '--- Forwarded references ---'
rg -n 'Forwarded|host=' pkg/fuzz .Repository: projectdiscovery/nuclei
Length of output: 5035
Encode the Forwarded probe as host=<canary>. Forwarded: <canary> is not RFC 7239 syntax, so strict parsers can ignore the probe and miss this host-reflection case.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/hostheader/analyzer.go` around lines 84 - 95, Update the
probe construction in the loop over probes so the Forwarded header uses RFC 7239
syntax, setting its value to host=<canary> rather than the bare canary. Keep the
existing direct Host assignment and other override-header behavior unchanged.
| resp, err := options.HttpClient.Do(rebuilt) | ||
| if err != nil { | ||
| // A transport error is expected when redirects are followed to the | ||
| // unresolvable canary host; skip this probe rather than aborting. | ||
| continue |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify redirect policy wiring and all affected analyzer call paths.
rg -n -C4 --type=go 'CheckRedirect|FollowRedirect|DisableRedirect|NoRedirect|HttpClient\.Do|DoAndReadBody' .
rg -n 'projectdiscovery/retryablehttp-go' go.mod go.sumRepository: projectdiscovery/nuclei
Length of output: 33027
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== redirect analyzer =="
sed -n '1,220p' pkg/fuzz/analyzers/redirect/analyzer.go
echo
echo "== hostheader analyzer =="
sed -n '1,220p' pkg/fuzz/analyzers/hostheader/analyzer.go
echo
echo "== analyzers helper =="
sed -n '1,240p' pkg/fuzz/analyzers/analyzers.goRepository: projectdiscovery/nuclei
Length of output: 14920
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== http client pool redirect handling =="
sed -n '430,560p' pkg/protocols/http/httpclientpool/clientpool.go
echo
echo "== redirect analyzer tests/e2e =="
sed -n '1,220p' pkg/fuzz/analyzers/e2e/e2e_test.goRepository: projectdiscovery/nuclei
Length of output: 13730
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== search for Location/header usage in fuzz analyzers =="
rg -n -C3 'Location|redirect' pkg/fuzz/analyzers -g '*.go'Repository: projectdiscovery/nuclei
Length of output: 17388
🌐 Web query:
retryablehttp-go Client Do redirect error response original response returned CheckRedirect semantics
💡 Result:
In hashicorp/go-retryablehttp, the library uses the underlying net/http Client for execution, which inherently respects the standard CheckRedirect semantics of the Go standard library [1][2]. When configuring a retryablehttp.Client, you can set a CheckRedirect function on the underlying HTTP client (via the HTTPClient field), which governs how the client handles HTTP redirects [3][4]. The semantics follow the standard net/http behavior: if the CheckRedirect function returns an error, the Client's Do method returns that error, and the original response (the one that triggered the redirect) is typically not returned in a successful manner, as the request is considered failed [5][6]. Key points regarding redirect behavior in this library include: - Redirect errors are often caught by the retry policy: The default DefaultRetryPolicy explicitly checks for redirect-related errors (e.g., too many redirects) and, in such cases, returns false to stop retrying and returns the error to the caller [7]. - Request Termination: If a redirect limit is reached or the CheckRedirect function returns an error, the attempt is aborted, and the error is bubbled up to the caller [7][6]. - Response Handling: Unlike standard status codes that might trigger a retry (like 500-range errors), a redirect error is treated as a request failure [7][4]. If you are using the library's Do method, the error returned will prevent the caller from receiving a valid *http.Response object, consistent with how the standard library treats errors returned by CheckRedirect [5][7]. If you need to access response data even when a redirect or other error occurs, ensure your logic does not rely on receiving a non-nil *http.Response alongside an error, as the client's behavior is designed to stop and return the error on failure [5][8]. Users have occasionally utilized a custom ErrorHandler to modify how terminal responses are returned, but the fundamental CheckRedirect contract remains bound to the standard library's implementation [5][8].
Citations:
- 1: https://pkg.go.dev/github.com/hashicorp/go-retryablehttp
- 2: https://godoc.org/github.com/hashicorp/go-retryablehttp
- 3: https://github.com/hashicorp/go-retryablehttp/blob/main/client.go
- 4: https://pkg.go.dev/github.com/hashicorp/go-retryablehttp@v0.7.8
- 5: "Get" returning error on non 2xx response hashicorp/go-retryablehttp#200
- 6: https://github.com/hashicorp/go-retryablehttp/blob/02c1586c8f14be23e7eeb522f1094afbabf45e93/client_test.go
- 7: https://github.com/hashicorp/go-retryablehttp/blob/master/client.go
- 8: fix: preserve final response in PassthroughErrorHandler hashicorp/go-retryablehttp#288
🌐 Web query:
Go http.Client Do CheckRedirect returns response and error semantics redirect follow failed original response
💡 Result:
In Go, the behavior of http.Client.CheckRedirect when returning an error depends on whether the error is the sentinel value http.ErrUseLastResponse or a standard error [1][2][3]. If CheckRedirect returns a standard error, the Client's request method (e.g., Get, Do) will return both a non-nil *http.Response (the previous response) and the returned error, which is wrapped in a *url.Error [1][3][4]. In this scenario, the Response body is already closed by the client before it is returned to the caller, preventing further reading of the response [1][2][3]. If CheckRedirect returns the sentinel value http.ErrUseLastResponse, the client stops following redirects, but it returns the most recent response with its body unclosed, along with a nil error [1][5][2]. This allows the caller to access the final response (the redirect response) without needing to handle a forced error, as the body remains open for reading [6][7][3]. Summary of semantics: 1. Standard error returned: Returns the previous response (Body closed) and the wrapped error [1][3]. 2. http.ErrUseLastResponse returned: Returns the previous response (Body open) and a nil error [1][2].
Citations:
- 1: https://pkg.go.dev/net/http@go1.26.4
- 2: https://go.googlesource.com/go/+/master/src/net/http/client.go
- 3: https://go.dev/src/net/http/client.go?m=text
- 4: https://stackoverflow.com/questions/27814942/selectively-follow-redirects-in-go
- 5: https://go.dev/src/net/http/client.go
- 6: https://pkg.go.dev/net/http
- 7: https://stackoverflow.com/questions/23297520/how-can-i-make-the-go-http-client-not-follow-redirects-automatically
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== full redirect analyzer =="
sed -n '1,220p' pkg/fuzz/analyzers/redirect/analyzer.go
echo
echo "== e2e redirect-related cases =="
rg -n -C4 'redirect|hostheader|Location|follow' pkg/fuzz/analyzers/e2e/e2e_test.goRepository: projectdiscovery/nuclei
Length of output: 7306
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== hostheader analyzer =="
sed -n '1,220p' pkg/fuzz/analyzers/hostheader/analyzer.go
echo
echo "== DoAndReadBody implementation =="
sed -n '130,170p' pkg/fuzz/analyzers/analyzers.goRepository: projectdiscovery/nuclei
Length of output: 5298
Preserve the redirect response on canary-follow failures.
When redirects are followed, a canary-targeted hop can fail after the server has already sent the 3xx response. Dropping that error path skips the Location/final-URL signal.
pkg/fuzz/analyzers/redirect/analyzer.go#L92-L96pkg/fuzz/analyzers/hostheader/analyzer.go#L98-L100
📍 Affects 2 files
pkg/fuzz/analyzers/redirect/analyzer.go#L92-L96(this comment)pkg/fuzz/analyzers/hostheader/analyzer.go#L98-L100
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/redirect/analyzer.go` around lines 92 - 96, Update the
redirect-follow error handling so a non-nil response is preserved and processed
even when HttpClient.Do returns an error; only skip the probe when no response
exists. Apply this in pkg/fuzz/analyzers/redirect/analyzer.go lines 92-96 and
pkg/fuzz/analyzers/hostheader/analyzer.go lines 98-100, ensuring the retained
response continues through the existing Location/final-URL analysis.
| defer func() { | ||
| _ = gr.Component.SetValue(gr.Key, gr.OriginalValue) | ||
| }() |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Avoid state leakage by calling Rebuild() after restoring the component value.
The defer cleanup block successfully resets the payload using gr.Component.SetValue(...) but omits the gr.Component.Rebuild() call. Based on learnings, the Component interface is stateful, and calling Rebuild() immediately after SetValue() is required per request to ensure the component's internal state accurately reflects the reset value. Omitting this can leak the injected fuzz payload into subsequent analyzers running on the same request component.
pkg/fuzz/analyzers/sqli/analyzer.go#L106-L108: Add_, _ = gr.Component.Rebuild()in the defer block.pkg/fuzz/analyzers/ssrf/analyzer.go#L97-L99: Add_, _ = gr.Component.Rebuild()in the defer block.pkg/fuzz/analyzers/lfi/analyzer.go#L93-L95: Add_, _ = gr.Component.Rebuild()in the defer block.
🛠️ Proposed fix (e.g. for the SQLi analyzer)
defer func() {
_ = gr.Component.SetValue(gr.Key, gr.OriginalValue)
+ _, _ = gr.Component.Rebuild()
}()📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| defer func() { | |
| _ = gr.Component.SetValue(gr.Key, gr.OriginalValue) | |
| }() | |
| defer func() { | |
| _ = gr.Component.SetValue(gr.Key, gr.OriginalValue) | |
| _, _ = gr.Component.Rebuild() | |
| }() |
📍 Affects 3 files
pkg/fuzz/analyzers/sqli/analyzer.go#L106-L108(this comment)pkg/fuzz/analyzers/ssrf/analyzer.go#L97-L99pkg/fuzz/analyzers/lfi/analyzer.go#L93-L95
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@pkg/fuzz/analyzers/sqli/analyzer.go` around lines 106 - 108, Restore each
component’s internal state after resetting its value by adding the
ignored-result Rebuild call immediately after SetValue in the defer cleanup
blocks for pkg/fuzz/analyzers/sqli/analyzer.go lines 106-108,
pkg/fuzz/analyzers/ssrf/analyzer.go lines 97-99, and
pkg/fuzz/analyzers/lfi/analyzer.go lines 93-95.
Source: Learnings
1 participant
Heuristic DAST analyzers that confirm findings beyond reflection matching. Closes #5839.
Summary by CodeRabbit
New Features
Bug Fixes