Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

net/http: http.FileServer returns 500 when a null byte is passed in the URL #72091

Closed
lodig-einride opened this issue Mar 4, 2025 · 5 comments
Labels
BugReport Issues describing a possible bug in the Go implementation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Milestone

Comments

@lodig-einride
Copy link

lodig-einride commented Mar 4, 2025

Go version

go version go1.24.0 darwin/arm64

Output of go env in your module/workspace:

AR='ar'
CC='clang'
CGO_CFLAGS='-O2 -g'
CGO_CPPFLAGS=''
CGO_CXXFLAGS='-O2 -g'
CGO_ENABLED='1'
CGO_FFLAGS='-O2 -g'
CGO_LDFLAGS='-O2 -g'
CXX='clang++'
GCCGO='gccgo'
GO111MODULE=''
GOARCH='arm64'
GOARM64='v8.0'
GOAUTH='netrc'
GOBIN=''
GOCACHE='/Users/greg/Library/Caches/go-build'
GOCACHEPROG=''
GODEBUG=''
GOENV='/Users/greg/Library/Application Support/go/env'
GOEXE=''
GOEXPERIMENT=''
GOFIPS140='off'
GOFLAGS=''
GOGCCFLAGS='-fPIC -arch arm64 -pthread -fno-caret-diagnostics -Qunused-arguments -fmessage-length=0 -ffile-prefix-map=/var/folders/02/s6w7nrhn6hl8_b1t1nslsfkh0000gp/T/go-build942617912=/tmp/go-build -gno-record-gcc-switches -fno-common'
GOHOSTARCH='arm64'
GOHOSTOS='darwin'
GOINSECURE=''
GOMOD='/Users/greg/Documents/yopass-500/go.mod'
GOMODCACHE='/Users/greg/go/pkg/mod'
GONOPROXY=''
GONOSUMDB=''
GOOS='darwin'
GOPATH='/Users/greg/go'
GOPRIVATE=''
GOPROXY='https://proxy.golang.org,direct'
GOROOT='/usr/local/go'
GOSUMDB='sum.golang.org'
GOTELEMETRY='local'
GOTELEMETRYDIR='/Users/greg/Library/Application Support/go/telemetry'
GOTMPDIR=''
GOTOOLCHAIN='auto'
GOTOOLDIR='/usr/local/go/pkg/tool/darwin_arm64'
GOVCS=''
GOVERSION='go1.24.0'
GOWORK=''
PKG_CONFIG='pkg-config'

What did you do?

Go's http.FileServer will return a 500 for requests that have a null byte character in the path, like http://localhost:8080/test%00.

The client is requesting to be served a file named test%00 but null bytes are not allowed in file names in any modern filesystems AFAIK.

Therefore I think the HTTP error code should be 400 (Bad Request) instead of 500 (Internal Server Error).

Repro code:

package main

import "net/http"

func main() {
	fileServer := http.FileServer(http.Dir("public"))

	http.HandleFunc("/", func(w http.ResponseWriter, r *http.Request) {
		fileServer.ServeHTTP(w, r)
	})

	http.ListenAndServe(":8080", nil)
}

What did you see happen?

❯ curl http://localhost:8080/test
404 page not found

❯ curl http://localhost:8080/test%00
500 Internal Server Error

What did you expect to see?

❯ curl http://localhost:8080/test
404 page not found

❯ curl http://localhost:8080/test%00
400 bad request
@lodig-einride
Copy link
Author

lodig-einride commented Mar 4, 2025

Interesting bits that did not fit the template:

We are interested in this change because we monitor services for 5xx error codes and every now and then an automated scanner will hit our services with a null byte in the URL which will trigger an alert for us.

Of course we could front our services with e.g. an nginx or WAF that would reject all requests with a null byte but I would rather fix the root cause and change the HTTP status code returned by Go http.FileServer to make it more appropriate than apply a patch like fronting all our services just for this use case.


Call stack:

http.serveFile (/usr/local/go/src/net/http/fs.go:685)
http.(*fileHandler).ServeHTTP (/usr/local/go/src/net/http/fs.go:986)
main.main.func1 (/Users/greg/Documents/yopass-500/main.go:11)

in http.serveFile, the call to fs.Open will return an error http: invalid or unsafe file path created in http.Dir.Open (/usr/local/go/src/net/http/fs.go:79).

This error is then not mapped to any specific error in http.toHTTPError (/usr/local/go/src/net/http/fs.go:764) and fall backs to the default 500 error.


My suggestion to implement a 400 instead of a 500:

  1. Define the error http: invalid or unsafe file path somewhere in the file
  2. Add an if clause to http.toHTTPError to map the error defined in 1 to HTTP status 400.

I can make a PR if this suggestion looks good 👍

Thank you

@gabyhelp gabyhelp added the BugReport Issues describing a possible bug in the Go implementation. label Mar 4, 2025
@JunyangShao
Copy link
Contributor

CC @neild

@JunyangShao JunyangShao added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Mar 4, 2025
@neild
Copy link
Contributor

neild commented Mar 4, 2025

I agree that we should return a 4xx error in this case, not a 500. I think 404 might make more sense than 400; this is just another case of an input path not mapping to a file we can serve.

Happy to review a CL if you want to send one.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/654975 mentions this issue: net/http: make http.FileServer return 404 when a path is invalid/unsafe

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. FixPending Issues that have a fix which has not yet been reviewed or submitted. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Mar 5, 2025
@dmitshur dmitshur added this to the Go1.25 milestone Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
BugReport Issues describing a possible bug in the Go implementation. FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants