fix(fuzz): concurrent map writes in multipart form parsing

[Mzack9999]

Proposed changes

Close #7028

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 of multipart form-data parsing by ensuring each concurrent request maintains its own independent decoder instance, preventing potential resource contention and race conditions.

• Tests

  • Added concurrency test coverage for multipart form-data parsing scenarios, validating correct operation under simultaneous requests.

[neo-by-projectdiscovery-dev]

Neo - PR Security Review

No security issues found

Highlights

• Fixes concurrent map write bug in multipart form parsing by creating per-request instances instead of using a shared singleton
• Adds encoder field to Value struct and properly clones it to maintain per-request state
• Includes comprehensive concurrency test with 20 goroutines to verify the fix

_{Comment @pdneo help for available commands. · Open in Neo}

[coderabbitai]

Walkthrough

This PR fixes a fatal concurrency issue in multipart form data fuzzing by instantiating a new decoder per request instead of reusing a shared singleton, storing the decoder instance for consistent use throughout request processing.

Changes

Cohort / File(s)	Summary
Multipart Form Data Decoder Instance Management pkg/fuzz/component/body.go	Modified parseBody to create a fresh dataformat.NewMultiPartForm() instance for each request when handling multipart/form-data (parsing boundary from Content-Type), rather than retrieving a shared singleton via dataformat.Get(). Stores the instantiated decoder into b.value.encoder to preserve state across processing.
Value Encoder Field & Conditional Encoding pkg/fuzz/component/value.go	Added encoder field to Value struct, ensured it persists through Clone(), and introduced helper method encode() that preferentially uses v.encoder.Encode() when available; otherwise falls back to dataformat.Encode(). Updated Encode() to use this conditional path for both ordered-map and nested-map encoding.
Concurrency Validation Test pkg/fuzz/component/body_test.go	Added TestMultiPartFormConcurrentParse spawning 20 concurrent goroutines, each independently creating multipart forms with unique boundaries, constructing HTTP requests, parsing via NewBody().Parse(req), fuzzing fields, and validating the rebuilt body contains expected content.

Sequence Diagram

sequenceDiagram
    participant G1 as Goroutine 1<br/>(Target A)
    participant G2 as Goroutine 2<br/>(Target B)
    participant Body as Body.parseBody()
    participant DF as dataformat
    participant Value as Value.encoder
    
    par Concurrent Requests
        G1->>Body: parseBody(req_A)
        G2->>Body: parseBody(req_B)
    end
    
    rect rgba(255, 100, 100, 0.5)
    Note over G1,G2: OLD (Race Condition)
    G1->>DF: Get("multipart/form-data")<br/>← returns singleton
    G2->>DF: Get("multipart/form-data")<br/>← same singleton!
    G1->>DF: singleton.ParseBoundary(contentType_A)<br/>write to boundary
    G2->>DF: singleton.ParseBoundary(contentType_B)<br/>write to boundary (DATA RACE)
    end
    
    rect rgba(100, 200, 100, 0.5)
    Note over G1,G2: NEW (Fixed)
    G1->>DF: NewMultiPartForm()<br/>← new instance
    G2->>DF: NewMultiPartForm()<br/>← new instance
    G1->>DF: decoder_A.ParseBoundary(contentType_A)
    G2->>DF: decoder_B.ParseBoundary(contentType_B)
    G1->>Value: encoder = decoder_A
    G2->>Value: encoder = decoder_B
    G1->>Value: encode() uses encoder_A
    G2->>Value: encode() uses encoder_B
    end

Loading

Estimated Code Review Effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Poem

🐰 Singleton shared, now threads collide,
Per-request decoders, each gets their own stride,
Encoders stored safe in Values so bright,
Concurrent fuzzing now works just right! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 50.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Linked Issues check	✅ Passed	The pull request successfully addresses issue #7028 by instantiating per-request multipart decoders, storing encoders in Value, and adding concurrent test coverage that validates the fix.
Out of Scope Changes check	✅ Passed	All changes directly address the concurrent map writes issue: per-request decoder instantiation, encoder storage/fallback logic, and concurrent test coverage for multipart parsing.
Title check	✅ Passed	The title 'fix(fuzz): concurrent map writes in multipart form parsing' directly addresses the root cause described in issue #7028—concurrent map writes in MultiPartForm.Decode during multipart/form-data fuzzing. The changes implement per-request decoder instances to prevent shared mutable state, which resolves the concurrent map write panic. The title is concise, specific, and accurately reflects the primary fix.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch 7028-fix-multipart-concurrent-map-writes

---

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

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

🧹 Nitpick comments (1)

pkg/fuzz/component/value.go (1)

41-48: Shallow copy of encoder shares mutable state between clones.

The Clone() method copies the encoder reference, meaning original and cloned Value instances share the same MultiPartForm pointer. While concurrent Encode() calls are safe (read-only on boundary/filesMetadata), this could become a latent issue if a caller re-parses the original while clones are in use.

If Clone() is used to spawn concurrent mutations of the same base request, consider creating a fresh encoder copy to fully isolate state. Given the typical usage pattern (each request gets its own Parse() call), this is low risk but worth noting for future-proofing.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@pkg/fuzz/component/value.go` around lines 41 - 48, Value.Clone currently
copies the encoder pointer, sharing Mutable MultiPartForm state between original
and clone; update Value.Clone so it deep-copies the encoder instead of copying
the reference — if v.encoder != nil create a new MultiPartForm (or call a
MultiPartForm.Clone method if available) and copy over its boundary and
filesMetadata (and any other internal fields) into the new encoder, then assign
that new encoder to the cloned Value; reference Value.Clone, the encoder field,
and the MultiPartForm type (or a new Clone method on it) when making this change
so clones are fully isolated from mutations to the original.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Nitpick comments:
In `@pkg/fuzz/component/value.go`:
- Around line 41-48: Value.Clone currently copies the encoder pointer, sharing
Mutable MultiPartForm state between original and clone; update Value.Clone so it
deep-copies the encoder instead of copying the reference — if v.encoder != nil
create a new MultiPartForm (or call a MultiPartForm.Clone method if available)
and copy over its boundary and filesMetadata (and any other internal fields)
into the new encoder, then assign that new encoder to the cloned Value;
reference Value.Clone, the encoder field, and the MultiPartForm type (or a new
Clone method on it) when making this change so clones are fully isolated from
mutations to the original.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: f7d64e70-40b7-4074-bbdb-e818e3a8a939

📥 Commits

Reviewing files that changed from the base of the PR and between e4edd5a and 8a4187b.

📒 Files selected for processing (3)

• pkg/fuzz/component/body.go
• pkg/fuzz/component/body_test.go
• pkg/fuzz/component/value.go