Skip to content

fix(pprofhandler): use exact path matching to prevent debug data exposure#2302

Merged
erikdubbelboer merged 3 commits into
valyala:masterfrom
xbrxr03:fix/pprof-exact-path-matching
Jul 4, 2026
Merged

fix(pprofhandler): use exact path matching to prevent debug data exposure#2302
erikdubbelboer merged 3 commits into
valyala:masterfrom
xbrxr03:fix/pprof-exact-path-matching

Conversation

@xbrxr03

@xbrxr03 xbrxr03 commented Jun 23, 2026

Copy link
Copy Markdown
Contributor

Summary

PprofHandler uses bytes.HasPrefix for routing, which means /debug/pprof/cmdlineFoo matches the cmdline handler. This leaks process command-line arguments (and other debug data) to any path that starts with a valid pprof endpoint prefix.

Fixes #2258.

What changed

pprofhandler/pprof.go

  • Replaced all bytes.HasPrefix route matches with matchPprofPath, which requires an exact path match or an exact match + trailing slash (matching net/http/pprof's DefaultServeMux behaviour)
  • The named-profile loop in the default branch also uses matchPprofPath instead of HasPrefix
  • Added matchPprofPath helper function and pre-computed endpoint byte slices

pprofhandler/pprof_test.go (new)

  • 20 tests covering exact matches, trailing slashes, prefix mismatches, and fallthrough behaviour
  • Verifies that /debug/pprof/cmdlineFoo no longer serves the process command line

Testing

All tests pass:

ok  github.com/valyala/fasthttp/pprofhandler  0.382s

Full repo test suite also passes — no regressions.

Security impact

Before: GET /debug/pprof/cmdlineFoo → serves full command line
After: GET /debug/pprof/cmdlineFoo → "Unknown profile" (falls through to index handler)

This aligns fasthttp's pprof handler with the Go standard library's exact-match behaviour on DefaultServeMux.

…sure

PprofHandler uses bytes.HasPrefix for routing, which means
/debug/pprof/cmdlineFoo matches the cmdline handler. This leaks
process command-line arguments to any path that starts with a valid
pprof endpoint prefix.

Replace HasPrefix with matchPprofPath, which requires an exact match
(or exact match + trailing slash, matching net/http/pprof's
DefaultServeMux behaviour). This prevents /debug/pprof/cmdlineFoo,
/debug/pprof/profileX, etc. from serving sensitive debug data.

Fixes valyala#2258
@erikdubbelboer

Copy link
Copy Markdown
Collaborator

Please fix the linting issues.

…ormatting, remove builtin shadow

- Remove unused pprofPrefix variable (gci unused linter)
- Fix gci import formatting in pprof.go and pprof_test.go
- Remove custom min() function that shadows predeclared builtin (gocritic builtinShadowDecl)
- Add missing trailing newline in pprof.go

Addresses maintainer review feedback on valyala#2302.
Comment thread pprofhandler/pprof.go Outdated
return true
}
// Allow trailing slash: /debug/pprof/heap/ matches /debug/pprof/heap
if len(path) == len(endpoint)+1 && path[len(path)-1] == '/' && bytes.Equal(path[:len(path)-1], endpoint) {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's match what net/http does, which returns a 404 for these paths ending in /

net/http/pprof does not register handlers for paths ending in /, so
requesting /debug/pprof/heap/ returns 404 via the default mux.

Update matchPprofPath to use strict equality only (no trailing slash
matching) and add an early 404 check in PprofHandler for any path
ending in /.

Renames test from AcceptsTrailingSlash to RejectsTrailingSlash and
updates matchPprofPath test to expect false for trailing slash.
@xbrxr03

xbrxr03 commented Jun 30, 2026

Copy link
Copy Markdown
Contributor Author

Updated: matchPprofPath now uses strict equality only (removed trailing-slash matching). PprofHandler returns 404 for any path ending in /, matching net/http/pprof behavior. Also renamed the test from AcceptsTrailingSlash to RejectsTrailingSlash and updated the matchPprofPath test case to expect false for trailing slash.

All pprofhandler tests pass locally.

@erikdubbelboer erikdubbelboer merged commit bba7659 into valyala:master Jul 4, 2026
11 checks passed
@erikdubbelboer

Copy link
Copy Markdown
Collaborator

Thanks!

erikdubbelboer pushed a commit that referenced this pull request Jul 5, 2026
* refactor: improve internal interface names

* test: fix host comparison in FuzzURIParse (#2313)

The fuzzer compares the parsed host against net/url and lowercases the
input to hide that fasthttp normalizes the host while net/url does not.
That trick cannot reach bytes hidden behind percent-encoding: "%4c"
survives ToLower and only decodes to 'L' afterwards, so fasthttp
returns "l" where net/url keeps "L". Found by the cifuzz job on an
unrelated pull request.

Lower the expectation instead, byte-wise via lowercaseBytes, since
strings.ToLower would mangle non-UTF-8 bytes like a decoded "%80".
Both hosts are added as seeds.

* perf: avoid redundant scans when parsing request headers (#2312)

* perf: avoid redundant scans when parsing request headers

RequestHeader.parse walks the header block twice before the lines are
actually parsed: readRawHeaders looks for the end of the block, then
headerScanner searches for the CRLFCRLF terminator once more. The
scanner now reuses the block end that readRawHeaders already found,
guarded by a check that the block really ends in CRLFCRLF. Blocks with
bare LF terminators fail that check and take the old search path, so
those odd cases keep behaving exactly as before.

While at it:

- trim header values with a plain loop, bytes.TrimLeft rebuilds its
  ASCII set on every call for the " \t" cutset
- drop the bytes.IndexByte(key, ' ') checks in front of
  normalizeHeaderKey, it already refuses keys with invalid bytes
  such as spaces
- simplify isValidHeaderKey, the noCanon flag changed nothing

benchstat (Apple M2 Pro, n=10):

RequestHeaderRead-12    79.48n ± 1%   63.22n ± 3%   -20.46% (p=0.000)
ServerGet1ReqPerConn-12 1.693µ ± 3%   1.671µ ± 1%   ~ (p=0.123)

* perf: reuse scanner work when parsing header lines

readContinuedLineSlice already scans every header line for the colon,
so hand that position to next() instead of finding it again with
bytes.Cut. Lines handed out there can never start with a space or tab
(the first line is rejected by the scanner, later ones are joined into
the previous header as continuations), so trimming never shifts the
position.

isValidHeaderKey now also reports whether the key contains a space that
survives trailing-whitespace trimming. The per-byte key loops in
parseHeaders and parseTrailer only existed to derive exactly that flag,
their reject branches were unreachable since the scanner validates
first. The response quirk that a space in a key closes the connection
is kept.

readLine loses its searchStart loop: the scanner truncates the block at
the terminator, so every line is guaranteed to end in \n.

benchstat (Apple M2 Pro, n=10), on top of the previous commit:

RequestHeaderRead-12    63.22n ± 3%   56.06n ± 1%   -11.33% (p=0.000)

and against master:

RequestHeaderRead-12    79.48n ± 1%   56.06n ± 1%   -29.48% (p=0.000)

* Fix temp file leak in SaveMultipartFile on cross-device rename failure (#2311)

When os.Rename fails (e.g., /tmp and destination on different filesystems),
the copy fallback path leaves the original multipart temp file orphaned
in /tmp. Add defer os.Remove(ff.Name()) to clean it up after copying.

* re-enable forcetypeassert (#2316)

* test: normalize Go test names (#2317)

* feat: prefix sentinel error strings (#2319)

* fix(pprofhandler): use exact path matching to prevent debug data exposure (#2302)

* fix(pprofhandler): use exact path matching to prevent debug data exposure

PprofHandler uses bytes.HasPrefix for routing, which means
/debug/pprof/cmdlineFoo matches the cmdline handler. This leaks
process command-line arguments to any path that starts with a valid
pprof endpoint prefix.

Replace HasPrefix with matchPprofPath, which requires an exact match
(or exact match + trailing slash, matching net/http/pprof's
DefaultServeMux behaviour). This prevents /debug/pprof/cmdlineFoo,
/debug/pprof/profileX, etc. from serving sensitive debug data.

Fixes #2258

* fix(pprofhandler): resolve lint issues — remove unused var, fix gci formatting, remove builtin shadow

- Remove unused pprofPrefix variable (gci unused linter)
- Fix gci import formatting in pprof.go and pprof_test.go
- Remove custom min() function that shadows predeclared builtin (gocritic builtinShadowDecl)
- Add missing trailing newline in pprof.go

Addresses maintainer review feedback on #2302.

* fix(pprofhandler): return 404 for paths with trailing slash

net/http/pprof does not register handlers for paths ending in /, so
requesting /debug/pprof/heap/ returns 404 via the default mux.

Update matchPprofPath to use strict equality only (no trailing slash
matching) and add an early 404 check in PprofHandler for any path
ending in /.

Renames test from AcceptsTrailingSlash to RejectsTrailingSlash and
updates matchPprofPath test to expect false for trailing slash.

---------

Co-authored-by: xbrxr03 <[email protected]>

* write fs compressed cache with os.CreateTemp to block symlink follow (#2321)

* refactor: improve internal interface names

* requestStreamHeader -> bodyStreamHeader

---------

Co-authored-by: RW <[email protected]>
Co-authored-by: artboy <[email protected]>
Co-authored-by: Md Abrar Ibn Habib <[email protected]>
Co-authored-by: xbrxr03 <[email protected]>
Co-authored-by: alhuda <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

security: pprofhandler imprecise path matching exposes debug data

3 participants