fix(engine): resolve memory and goroutine leaks in embedded engine usage (#7503) - #7508
Closed
ThryLox wants to merge 26 commits into
Closed
fix(engine): resolve memory and goroutine leaks in embedded engine usage (#7503)#7508ThryLox wants to merge 26 commits into
ThryLox wants to merge 26 commits into
Conversation
…iscovery#7455) * fix(hosterrorscache): skip hosts that consistently time out Count ErrKindNetworkTemporary (request timeouts) toward MaxHostError so a host that times out on every request is skipped, instead of being probed by every template. Reset-on-success still protects slow-but-alive hosts, so only consecutive timeouts with no successful response reach the threshold. Closes projectdiscovery#7454 * fix(hosterrorscache): count rawhttp i/o timeouts toward host skip rawhttp/unsafe templates surface read timeouts as a plain-string error ("ReadStatusLine: ... i/o timeout") that errkit cannot classify, so it reaches the regex fallback. The regex did not list i/o timeout, so a host that timed out on every rawhttp request was never skipped. Add i/o timeout to the pattern. Verified: 30 unsafe templates vs one unresponsive host went from 271s with no skip to skipping after the threshold. * fix(hosterrorscache): reset on success and ignore parent-context cancellation Addresses review feedback on the timeout-skip change: - The non-clustered HTTP path never reset the host-errors cache on a successful response (markHostError ignores nil, success branch only incremented progress), so timeout counts were not consecutive and a live host with intermittent timeouts could be skipped. Add markHostSuccess and call it on a successful request, mirroring the clustered path. - context.DeadlineExceeded classifies as ErrKindNetworkTemporary, so a failure from the parent scan context being cancelled/deadlined would count as a host failure. Ignore failures in MarkFailedOrRemove when the caller's context is already done; success still resets. Tests: parent-context cancellation not counted, non-consecutive timeouts do not skip, and the HTTP path resets the cache on success. * fix(hosterrorscache): reset host cache on success across all HTTP paths The timeout-counting change is global (in the cache), but reset-on-success was only wired into the sequential HTTP path. The parallel, race and pipeline paths counted failures without resetting on success, so a live host with intermittent timeouts could be falsely skipped in those modes. Consolidate mark-and-reset into recordHostResult and call it from every HTTP execution path (sequential, parallel, race, pipeline). The SPM handler invokes the result callback for every request outcome including success, so resetting there is sufficient. * test(http): add test for the parallel generated-host case Signed-off-by: Dwi Siswanto <git@dw1.io> * fix(http): use generated input when record host errors The async HTTP paths checked generated requests with the updated input, but recorded host-error results with the original input. If a generated URL changed the effective host/port, `Check()` and `MarkFailedOrRemove()` operated on different cache keys. So record race, parallel, and pipeline results with the same updated input used for the request to keep timeout counts and success resets attached to the host that was actually checked. Signed-off-by: Dwi Siswanto <git@dw1.io> --------- Signed-off-by: Dwi Siswanto <git@dw1.io> Co-authored-by: Dwi Siswanto <git@dw1.io>
…7463) Bumps the modules group with 3 updates: [github.com/projectdiscovery/retryablehttp-go](https://github.com/projectdiscovery/retryablehttp-go), [github.com/projectdiscovery/wappalyzergo](https://github.com/projectdiscovery/wappalyzergo) and [github.com/projectdiscovery/cdncheck](https://github.com/projectdiscovery/cdncheck). Updates `github.com/projectdiscovery/retryablehttp-go` from 1.3.14 to 1.3.15 - [Release notes](https://github.com/projectdiscovery/retryablehttp-go/releases) - [Commits](projectdiscovery/retryablehttp-go@v1.3.14...v1.3.15) Updates `github.com/projectdiscovery/wappalyzergo` from 0.2.84 to 0.2.85 - [Release notes](https://github.com/projectdiscovery/wappalyzergo/releases) - [Commits](projectdiscovery/wappalyzergo@v0.2.84...v0.2.85) Updates `github.com/projectdiscovery/cdncheck` from 1.2.39 to 1.2.40 - [Release notes](https://github.com/projectdiscovery/cdncheck/releases) - [Commits](projectdiscovery/cdncheck@v1.2.39...v1.2.40) --- updated-dependencies: - dependency-name: github.com/projectdiscovery/retryablehttp-go dependency-version: 1.3.15 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: modules - dependency-name: github.com/projectdiscovery/wappalyzergo dependency-version: 0.2.85 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: modules - dependency-name: github.com/projectdiscovery/cdncheck dependency-version: 1.2.40 dependency-type: indirect update-type: version-update:semver-patch dependency-group: modules ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
* attempting per-host http client pool * close idle conns * fixing idle conn leak * lint * ignore http internal routines * fixing tests * lowering flaky http requirement * fixing open gate issue * adding perf tests * pool rework
…tdiscovery#7323) (projectdiscovery#7465) * fix: preserve explicit target port in network templates (fixes projectdiscovery#7323) Problem ------- When a user specifies a port explicitly on the command line, e.g.: nuclei -target TARGET:80 -t network/cves/2001/CVE-2001-1473.yaml UseNetworkPort() was overriding port 80 with the template's port (22). This happened because reservedPorts (80, 443, 8080, …) were always replaced by the template port, regardless of whether the port was deliberately chosen by the operator or merely implied by the URL scheme. As a result, services running on non-standard ports (SSH on 80, FTP on 443, etc.) were silently scanned on the wrong port, or the connection was refused, and the target was effectively invisible to Nuclei. Root cause ---------- UseNetworkPort() treated a bare 'host:80' the same as 'http://host:80'. In the former the operator explicitly chose port 80; in the latter the port was implied by the http:// scheme. Fix --- Only replace a reserved port when the input contains a URL scheme ('://'), indicating the port was scheme-implied. A bare 'host:port' form means the operator deliberately chose that port and it is preserved unchanged. Regression tests ---------------- Seven table-driven cases added to contextargs_test.go covering: - No port in input → template port used (existing behaviour) - Explicit non-reserved port → preserved - Bare host:80 (key regression) → preserved - http://host:80 (scheme-implied) → replaced - http://host (no port) → template port used - Empty template port → no-op - Explicit port == template port → unchanged Fixes projectdiscovery#7323 * expand tests * fix tests --------- Co-authored-by: XananasX7 <xananasX7@users.noreply.github.com> Co-authored-by: Mzack9999 <mzack9999@protonmail.com>
…8 input (projectdiscovery#7464) Upgrade github.com/projectdiscovery/govaluate from v0.0.0-20260504230327-80320480bb6e to v0.0.0-20260615100919-5ee2581bbf7e to consume the fix merged in projectdiscovery/govaluate#4. The govaluate lexer advanced strPosition by utf8.RuneLen(utf8.RuneError) == 3 for invalid bytes, even though only 1 byte was consumed. This caused byte-offset drift past the actual string length and a `slice bounds out of range` panic in readUntilFalse. Nuclei's expressions.Evaluate only caught govaluate errors, not panics, so targets returning invalid UTF-8 in response data could crash the entire process. Fixes projectdiscovery#7462
* . * . * adding pre-flight * add conn reuse * making pre-flight optional * adding per-host httpclient + ratelimit * add conn reuse stats * ignoring leak in lru * adding global connection pooling with sharding * lint * addressing comments * fix lint: disambiguate retryablehttp URL access retryablehttp.Request embeds *urlutil.URL, so .URL.String() resolved to the wrong type and tripped staticcheck QF1008. Use .Request.URL to read the underlying http.Request URL for rate limiting and pooling keys. * fix race * regression harness * ci tuning * address review comments - preflight: resolve via MetaInput.Target()/CustomIP, gate banner on Silent - http: drop dead Priority 4 branch in AnalyzeConnectionReuse * add tests * host stats * reuse timing --------- Co-authored-by: Ice3man <nizamulrana@gmail.com>
Hoist the unsigned code-template guard ahead of the dast branch so it applies on every load path.
Route every mysql DSN through a sandbox that drops allowAllFiles unless -lfa is set, matching the fs.ReadFile restriction.
* chore(deps): strip dependencies Signed-off-by: Dwi Siswanto <git@dw1.io> * chore: drop stripped dependencies Signed-off-by: Dwi Siswanto <git@dw1.io> * tests(integration): reduce external service flakiness Signed-off-by: Dwi Siswanto <git@dw1.io> * refactor(types): replace govalidator ASCII check Signed-off-by: Dwi Siswanto <git@dw1.io> * chore(trackers): update go-github import path Signed-off-by: Dwi Siswanto <git@dw1.io> * chore: use our PD wrapper Signed-off-by: Dwi Siswanto <git@dw1.io> * chore: use our PD cache packages Signed-off-by: Dwi Siswanto <git@dw1.io> * chore(utils): use x/net publicsuffix Signed-off-by: Dwi Siswanto <git@dw1.io> * chore(input): decode YAML without invopop/yaml Signed-off-by: Dwi Siswanto <git@dw1.io> * refactor(yaml): route callers through pkg/utils/yaml Signed-off-by: Dwi Siswanto <git@dw1.io> * refactor(json): route callers through pkg/utils/json Signed-off-by: Dwi Siswanto <git@dw1.io> * refactpr(scan): replace Echo handlers with net/http Signed-off-by: Dwi Siswanto <git@dw1.io> * refactor(server): replace Echo with net/http Signed-off-by: Dwi Siswanto <git@dw1.io> * refactor(fuzzplayground): replace Echo server with net/http Signed-off-by: Dwi Siswanto <git@dw1.io> * test: update subdomains testdata Signed-off-by: Dwi Siswanto <git@dw1.io> * chore(yaml): satisfy lints Signed-off-by: Dwi Siswanto <git@dw1.io> * fix codeql --------- Signed-off-by: Dwi Siswanto <git@dw1.io> Co-authored-by: Mzack9999 <mzack9999@protonmail.com>
`highlightAsciiSection` built a regex like `(.\n*)(.\n*)` for every non-printable byte in a matcher snippet, intending the `.` as the literal character hex.Dump puts in the ASCII column. The regex engine treated it as "any char". On the first pass this over-highlights every character in the row; on the second highlight pass (when more than one matcher hit), the now ANSI-coloured row is full of 2-character fragments like "\x1b[", "32", "0m" that all match. Each strings.ReplaceAll in the highlight loop then inflates the buffer 3-10x, producing exponential growth that pegs CPU and OOMs the process within seconds. Triggered by any TCP/network template with >=2 binary matchers of different lengths over a response containing non-printable bytes, run with -debug / -debug-resp / -store-resp. Adds a regression test that fails (hangs >5s) on the bug and passes in <10ms with the fix.
…projectdiscovery#7491) Bumps the workflows group with 1 update: [actions/checkout](https://github.com/actions/checkout). Updates `actions/checkout` from 6 to 7 - [Release notes](https://github.com/actions/checkout/releases) - [Changelog](https://github.com/actions/checkout/blob/main/CHANGELOG.md) - [Commits](actions/checkout@v6...v7) --- updated-dependencies: - dependency-name: actions/checkout dependency-version: '7' dependency-type: direct:production update-type: version-update:semver-major dependency-group: workflows ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
…7490) Bumps the modules group with 3 updates: [github.com/projectdiscovery/gologger](https://github.com/projectdiscovery/gologger), [github.com/projectdiscovery/wappalyzergo](https://github.com/projectdiscovery/wappalyzergo) and [github.com/projectdiscovery/cdncheck](https://github.com/projectdiscovery/cdncheck). Updates `github.com/projectdiscovery/gologger` from 1.1.70 to 1.1.71 - [Release notes](https://github.com/projectdiscovery/gologger/releases) - [Commits](projectdiscovery/gologger@v1.1.70...v1.1.71) Updates `github.com/projectdiscovery/wappalyzergo` from 0.2.85 to 0.2.86 - [Release notes](https://github.com/projectdiscovery/wappalyzergo/releases) - [Commits](projectdiscovery/wappalyzergo@v0.2.85...v0.2.86) Updates `github.com/projectdiscovery/cdncheck` from 1.2.40 to 1.2.41 - [Release notes](https://github.com/projectdiscovery/cdncheck/releases) - [Commits](projectdiscovery/cdncheck@v1.2.40...v1.2.41) --- updated-dependencies: - dependency-name: github.com/projectdiscovery/gologger dependency-version: 1.1.71 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: modules - dependency-name: github.com/projectdiscovery/wappalyzergo dependency-version: 0.2.86 dependency-type: direct:production update-type: version-update:semver-patch dependency-group: modules - dependency-name: github.com/projectdiscovery/cdncheck dependency-version: 1.2.41 dependency-type: indirect update-type: version-update:semver-patch dependency-group: modules ... Signed-off-by: dependabot[bot] <support@github.com> Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
`PGClient.ExecuteQuery` built the connection URL by interpolating the dbname directly into the path. A value containing '?' could start the query string and inject lib/pq options such as `sslrootcert` before the appended `sslmode=disable`. Build the URL from escaped userinfo, path and query values so dbname stays part of the dbname. Signed-off-by: Dwi Siswanto <git@dw1.io>
…very#7489) Move template execution requirements into a shared capability table that maps each capability to its flag, stats key, and template predicate. Use the shared model from template loading, workflow parsing, request compilation, and runner stats display so file, self-contained, headless, code, DAST, and global matcher gates stay in sync. Signed-off-by: Dwi Siswanto <git@dw1.io>
…#7494) LDAP was the only JS protocol client that reached the dialer without an `IsHostAllowed` check. Validate ldap, ldaps, and cldap hosts before connecting, and treat ldapi as loopback so restricted local network access rejects Unix socket LDAP before the socket path is opened. Signed-off-by: Dwi Siswanto <git@dw1.io>
…covery#7480) go-ora accepts `TRACE {FILE,DIR}`-style DSN options and may create those files while opening the conn. Normalize those paths and reject outside paths unless `-allow-local-file-access` is enabled. Signed-off-by: Dwi Siswanto <git@dw1.io>
The MSSQL helpers put `dbName` directly in the URL query as the database parameter. A template could include '&' in the database name and append driver options such as certificate/Kerberos file paths. Build the URL through a shared helper and query- escape the database value so it stays part of the database parameter. Signed-off-by: Dwi Siswanto <git@dw1.io>
…ry#7482) `OutputFile` was treated as a goimpacket option instead of a fs sink. Since goimpacket writes the ccache itself, krbforge had to enforce nuclei's local file policy before building the ticket config. Add execution-aware JS wrappers, normalize relative output paths into the template sandbox, and deny outside paths unless local file access is enabled. Keep empty output as in-memory only by passing "-". Signed-off-by: Dwi Siswanto <git@dw1.io>
…very#7459) * fuzz: add parser harnesses for raw requests & templates Add go-fuzz harnesses for raw request and template parsing. The new coverage exercises input raw request parsing, HTTP raw request parsing across safe and unsafe modes, and YAML/JSON template parsing with lightweight compile-time validation. Seed corpora cover common ingestion shapes, host reconstruction, full URL paths, empty path automerge, relative query paths, and raw HTTP template requests. Part of projectdiscovery#7312 Signed-off-by: Dwi Siswanto <git@dw1.io> * fuzz(types): add fuzz coverage for raw request parsing Add a go-fuzz harness for `ParseRawRequest` and `ParseRawRequestWithURL`. The seed corpus covers common raw request ingestion shapes, including simple GET requests, form posts, JSON bodies, and URL override cases. Signed-off-by: Dwi Siswanto <git@dw1.io> * fuzz(http): add fuzz coverage for raw request modes Add a go-fuzz harness for `Parse` and `ParseRawRequest`. The corpus covers unsafe requests, self-contained host reconstruction, empty path handling, path automerge, full URL paths, and relative query paths. Signed-off-by: Dwi Siswanto <git@dw1.io> * fuzz(templates): add fuzz coverage for parsing & compile checks Add a go-fuzz harness for YAML and JSON template parsing. Generated templates are parsed in both formats and passed thru lightweight compile-time validation. Helper file loading is disabled so fuzz inputs cannot turn the compile check into filesystem access, and no protocol execution is run. Signed-off-by: Dwi Siswanto <git@dw1.io> * chore: satisfy lints Signed-off-by: Dwi Siswanto <git@dw1.io> --------- Signed-off-by: Dwi Siswanto <git@dw1.io>
) HTTP fuzzing does not maintain the ordered request history used by request-condition fields. Reject fuzzing templates that reference those fields at validation time instead of letting them eval with an invalid request count. Fixes projectdiscovery#4891 Signed-off-by: Dwi Siswanto <git@dw1.io>
Signed-off-by: Dwi Siswanto <git@dw1.io>
* fix(yaml): reject recursive include chains Track the include stack while expanding YAML includes and fail when a file is seen again or the nesting limit is reached, keeping circular includes from recursing until the process panics. Also handle includes at the start of the buffer, where the old padding lookup could slice from -1. Signed-off-by: Dwi Siswanto <git@dw1.io> * fix indentation * fix typo --------- Signed-off-by: Dwi Siswanto <git@dw1.io> Co-authored-by: Mzack9999 <mzack9999@protonmail.com>
…age (projectdiscovery#7503) Addresses memory and goroutine accumulation during long-running embedded engine usage as outlined in projectdiscovery#7503 / projectdiscovery#7502: 1. Replaced unbounded sync.Map in HTTPToHTTPSPortTracker with a size-bounded expirable LRU cache (4096 entries max, 24h TTL) and added Purge(). 2. Updated protocolstate.Close() to release per-host rate-limit pool goroutines and purge HTTP-to-HTTPS tracker entries. 3. Updated NucleiEngine.Close() / closeInternal() to purge parsed and compiled template caches on engine shutdown.
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #7503 by implementing the required leak-prevention cleanup mechanisms outlined in #7502 for long-running embedded engines.
Key Changes
sync.MapinHTTPToHTTPSPortTracker(pkg/protocols/http/httpclientpool/http_to_https_tracker.go) with a size-bounded expirable LRU cache (4096 entries max, 24h TTL) and addedPurge().protocolstate.Close()(pkg/protocols/common/protocolstate/state.go) to release per-host rate-limit pool goroutines and purge the HTTP-to-HTTPS tracker on shutdown.NucleiEngine.Close()/closeInternal()(lib/sdk.go) andParser(pkg/templates/parser.go) to purge parsed and compiled template caches on engine close.