Address race conditions in http.Request and MemGuardian #6321

hdm · 2025-07-15T07:44:26Z

Proposed changes

This PR addresses two sets of race conditions:

An active HTTP scan could race against the dump/curl functions (same engine)
A global race in MemGuardian (concurrent engines)

Example traces:

HTTP

==================
WARNING: DATA RACE
Write at 0x00c020e49580 by goroutine 1527061:
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/http.(*Request).executeRequest()
      project/nuclei/pkg/protocols/http/request.go:869 +0x7170
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/http.(*Request).ExecuteWithResults.func1()
      project/nuclei/pkg/protocols/http/request.go:508 +0xa04
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/http.(*Request).ExecuteWithResults()
      project/nuclei/pkg/protocols/http/request.go:585 +0x786
  github.com/projectdiscovery/nuclei/v3/pkg/tmplexec/generic.(*Generic).ExecuteWithResults()
      project/nuclei/pkg/tmplexec/generic/exec.go:61 +0x595
  github.com/projectdiscovery/nuclei/v3/pkg/tmplexec.(*TemplateExecuter).Execute()
      project/nuclei/pkg/tmplexec/exec.go:216 +0x876
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateWithTargets.func2.1()
      project/nuclei/pkg/core/executors.go:138 +0x415
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateWithTargets.func2.gowrap1()
      project/nuclei/pkg/core/executors.go:145 +0x61

Previous read at 0x00c020e49580 by goroutine 1605854:
  net/http.newTransferWriter()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transfer.go:93 +0x3b3
  net/http.(*Request).write()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/request.go:706 +0x1011
  net/http.(*persistConn).writeLoop()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transport.go:2593 +0x364
  net/http.(*Transport).dialConn.gowrap3()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transport.go:1945 +0x33

Goroutine 1527061 (running) created at:
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateWithTargets.func2()
      project/nuclei/pkg/core/executors.go:114 +0xd6a
  github.com/projectdiscovery/nuclei/v3/pkg/input/provider.(*SimpleInputProvider).Iterate()
      project/nuclei/pkg/input/provider/simple.go:38 +0x8a
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateWithTargets()
      project/nuclei/pkg/core/executors.go:79 +0x85b
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateSpray.func1()
      project/nuclei/pkg/core/execute_options.go:137 +0x10a
  github.com/projectdiscovery/nuclei/v3/pkg/core.(*Engine).executeTemplateSpray.gowrap2()
      project/nuclei/pkg/core/execute_options.go:138 +0x41

Goroutine 1605854 (running) created at:
  net/http.(*Transport).dialConn()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transport.go:1945 +0x2dd9
  net/http.(*Transport).dialConnFor()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transport.go:1615 +0x11d
  net/http.(*Transport).startDialConnForLocked.func1()
      /root/go/pkg/mod/golang.org/[email protected]/src/net/http/transport.go:1597 +0x3c
==================

MemGuardian

==================
WARNING: DATA RACE
Read at 0x0000111e4aa8 by goroutine 795469:
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.StartActiveMemGuardian()
      project/nuclei/pkg/protocols/common/protocolstate/memguardian.go:22 +0x47
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.initDialers()
      project/nuclei/pkg/protocols/common/protocolstate/state.go:201 +0x17c8
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.Init()
      project/nuclei/pkg/protocols/common/protocolstate/state.go:61 +0xb1
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolinit.Init()
      project/nuclei/pkg/protocols/common/protocolinit/init.go:17 +0x2e
  github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).init()
      project/nuclei/lib/sdk_private.go:120 +0x5b8
  github.com/projectdiscovery/nuclei/v3/lib.NewNucleiEngineCtx()
      project/nuclei/lib/sdk.go:326 +0x4b6
  github.com/runZeroInc/platform/runzero.(*Scanner).vulnScanTechDetectionHTTP()
      /home/runzero/platform/runzero/scanner_vscan.go:1257 +0x1ef9
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpAnalyzeActive()
      /home/runzero/platform/runzero/tcp_http_analyze_active.go:310 +0x766e
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpProcessResponse()
      /home/runzero/platform/runzero/tcp_http.go:1115 +0x1dc4
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpParseRaw()
      /home/runzero/platform/runzero/tcp_http.go:504 +0x97e
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessResponse()
      /home/runzero/platform/runzero/tcp.go:2666 +0x70d2
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ActiveFingerprint()
      /home/runzero/platform/runzero/tcp.go:3598 +0x3e54
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).GatherInformation()
      /home/runzero/platform/runzero/tcp.go:3180 +0x22b6
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessTarget()
      /home/runzero/platform/runzero/tcp.go:961 +0x7c4
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessSingleTarget.func2()
      /home/runzero/platform/runzero/tcp.go:650 +0x204

Previous write at 0x0000111e4aa8 by goroutine 795470:
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.StartActiveMemGuardian()
      project/nuclei/pkg/protocols/common/protocolstate/memguardian.go:26 +0x85
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.initDialers()
      project/nuclei/pkg/protocols/common/protocolstate/state.go:201 +0x17c8
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolstate.Init()
      project/nuclei/pkg/protocols/common/protocolstate/state.go:61 +0xb1
  github.com/projectdiscovery/nuclei/v3/pkg/protocols/common/protocolinit.Init()
      project/nuclei/pkg/protocols/common/protocolinit/init.go:17 +0x2e
  github.com/projectdiscovery/nuclei/v3/lib.(*NucleiEngine).init()
      project/nuclei/lib/sdk_private.go:120 +0x5b8
  github.com/projectdiscovery/nuclei/v3/lib.NewNucleiEngineCtx()
      project/nuclei/lib/sdk.go:326 +0x4b6
  github.com/runZeroInc/platform/runzero.(*Scanner).vulnScanTechDetectionHTTP()
      /home/runzero/platform/runzero/scanner_vscan.go:1257 +0x1ef9
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpAnalyzeActive()
      /home/runzero/platform/runzero/tcp_http_analyze_active.go:310 +0x766e
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpProcessResponse()
      /home/runzero/platform/runzero/tcp_http.go:1115 +0x1dc4
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).httpParseRaw()
      /home/runzero/platform/runzero/tcp_http.go:504 +0x97e
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessResponse()
      /home/runzero/platform/runzero/tcp.go:2666 +0x70d2
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ActiveFingerprint()
      /home/runzero/platform/runzero/tcp.go:3598 +0x3e54
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).GatherInformation()
      /home/runzero/platform/runzero/tcp.go:3180 +0x22b6
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessTarget()
      /home/runzero/platform/runzero/tcp.go:961 +0x7c4
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessSingleTarget.func2()
      /home/runzero/platform/runzero/tcp.go:650 +0x204

Goroutine 795469 (running) created at:
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessSingleTarget()
      /home/runzero/platform/runzero/tcp.go:642 +0x512
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessTargets()
      /home/runzero/platform/runzero/tcp.go:596 +0x1e4
  github.com/runZeroInc/platform/runzero.NewTCPPortScanner.gowrap1()
      /home/runzero/platform/runzero/tcp.go:218 +0x33

Goroutine 795470 (running) created at:
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessSingleTarget()
      /home/runzero/platform/runzero/tcp.go:642 +0x512
  github.com/runZeroInc/platform/runzero.(*TCPPortScanner).ProcessTargets()
      /home/runzero/platform/runzero/tcp.go:596 +0x1e4
  github.com/runZeroInc/platform/runzero.NewTCPPortScanner.gowrap1()
      /home/runzero/platform/runzero/tcp.go:218 +0x33
==================

Checklist

Pull request is created against the dev branch
All checks passed (lint, unit/integration/regression tests etc.) with my changes
I have added tests that prove my fix is effective or that my feature works
I have added necessary documentation (if appropriate)

Summary by CodeRabbit

Bug Fixes
- Improved thread safety and reliability when handling HTTP requests, reducing the risk of race conditions during request processing and logging.
Chores
- Enhanced internal synchronization mechanisms for memory management processes.

coderabbitai · 2025-07-15T07:44:33Z

Walkthrough

The changes introduce improved concurrency safety in memory guardian functions by adjusting mutex placement and usage. Additionally, HTTP request handling is updated to use cloned requests when generating curl commands and dumping requests, preventing race conditions by avoiding in-place modifications of original request objects.

Changes

File(s)	Change Summary
pkg/protocols/common/protocolstate/memguardian.go	Moved and initialized `muGlobalChange` mutex at package level; updated locking in two functions.
pkg/protocols/http/request.go	Modified curl command generation to use a cloned request, avoiding in-place modification.
pkg/protocols/http/utils.go	Updated `dump` function to operate on a cloned request to prevent race conditions.

Suggested reviewers

dogancanbakir

Poem

In the warren of code, where races may hide,
Mutexes now guard, with safety applied.
HTTP requests, cloned with great care,
No more collisions or bugs to ensnare.
The rabbit hops on, with a satisfied cheer,
For safer code is finally here! 🐇✨

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 1079498 and 875941c.

📒 Files selected for processing (3)

pkg/protocols/common/protocolstate/memguardian.go (2 hunks)
pkg/protocols/http/request.go (1 hunks)
pkg/protocols/http/utils.go (1 hunks)

🧰 Additional context used

🧠 Learnings (2)

pkg/protocols/http/request.go (1)

Learnt from: dwisiswant0
PR: projectdiscovery/nuclei#6290
File: pkg/protocols/http/build_request.go:457-464
Timestamp: 2025-06-30T16:34:42.125Z
Learning: In the projectdiscovery/retryablehttp-go package, the Request struct embeds URL fields directly, making req.Scheme, req.Host, and other URL fields accessible directly on the Request object instead of requiring req.URL.Scheme, req.URL.Host, etc.

pkg/protocols/http/utils.go (1)

Learnt from: dwisiswant0
PR: projectdiscovery/nuclei#6290
File: pkg/protocols/http/build_request.go:457-464
Timestamp: 2025-06-30T16:34:42.125Z
Learning: In the projectdiscovery/retryablehttp-go package, the Request struct embeds URL fields directly, making req.Scheme, req.Host, and other URL fields accessible directly on the Request object instead of requiring req.URL.Scheme, req.URL.Host, etc.

🔇 Additional comments (5)

pkg/protocols/http/utils.go (1)

15-16: LGTM: Effective race condition fix for HTTP request dumping.

The change to clone the request before dumping is a proper solution to prevent race conditions with the HTTP transport. The added comment clearly explains the purpose, and the implementation correctly preserves the original request context.

pkg/protocols/common/protocolstate/memguardian.go (3)

19-19: LGTM: Proper mutex declaration for global synchronization.

The mutex is correctly declared at the package level to provide synchronization for global memory guardian operations.

23-24: LGTM: Correct mutex protection for StartActiveMemGuardian.

The lock/unlock pattern with defer is properly implemented to prevent race conditions during memory guardian initialization. This addresses the global race condition mentioned in the PR objectives.

48-50: LGTM: Proper mutex protection for StopActiveMemGuardian.

The mutex protection is correctly implemented to prevent race conditions during memory guardian shutdown. The lock/unlock pattern matches the style used in the start function.

pkg/protocols/http/request.go (1)

867-870: LGTM: Consistent race condition fix for curl command generation.

The implementation correctly clones the HTTP request before setting the body and generating the curl command. This prevents race conditions with the HTTP transport and aligns with the similar fix in utils.go. The approach of using Clone(resp.Request.Context()) preserves the original request context while avoiding in-place modifications.

✨ Finishing Touches

📝 Generate Docstrings

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.

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Explain this complex logic.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai explain this code block.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and explain its main purpose.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Support

Need help? Create a ticket on our support page for assistance with any issues or questions.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

Mzack9999

lgtm - mux + clones seems a good approach to prevent races from happening

hdm added 2 commits July 15, 2025 02:34

avoid data races by using request clones

6bf3f14

avoid data races using mutex for memguardian

875941c

auto-assign bot requested a review from dogancanbakir July 15, 2025 07:44

hdm mentioned this pull request Jul 15, 2025

Fix data race caused by concurrent access to the request URL during scans projectdiscovery/retryablehttp-go#448

Merged

Mzack9999 self-requested a review July 15, 2025 13:17

Mzack9999 approved these changes Jul 15, 2025

View reviewed changes

Mzack9999 merged commit 3e9bee7 into projectdiscovery:dev Jul 15, 2025
20 checks passed

coderabbitai bot mentioned this pull request Jul 16, 2025

Support concurrent Nuclei engines in the same process #6322

Merged

4 tasks

BrewTestBot mentioned this pull request Aug 21, 2025

nuclei 3.4.8 Homebrew/homebrew-core#234422

Merged

blsaccess mentioned this pull request Aug 22, 2025

Update nuclei to 3.4.8 blacklanternsecurity/bbot#2623

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Address race conditions in http.Request and MemGuardian #6321

Address race conditions in http.Request and MemGuardian #6321

Uh oh!

hdm commented Jul 15, 2025 •

edited by coderabbitai bot

Loading

Uh oh!

coderabbitai bot commented Jul 15, 2025 •

edited

Loading

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

Mzack9999 left a comment

Uh oh!

Uh oh!

Uh oh!

Address race conditions in http.Request and MemGuardian #6321

Address race conditions in http.Request and MemGuardian #6321

Uh oh!

Conversation

hdm commented Jul 15, 2025 • edited by coderabbitai bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Proposed changes

Checklist

Summary by CodeRabbit

Uh oh!

coderabbitai bot commented Jul 15, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Suggested reviewers

Poem

Chat

Support

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Mzack9999 left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

hdm commented Jul 15, 2025 •

edited by coderabbitai bot

Loading

coderabbitai bot commented Jul 15, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)