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: ServeFile and FileServer information leak #46413
Comments
Be aware that the |
fs.FS can't know if the information it gives is gonna be exposed to attackers, while ServeFile does. ServeFile will always be exposed and it could be called by user code after parsing a request parameter, so it doesn't necessarily mean it comes from a http.Dir call. I think the right place to put this protection in is as close as possible to where the error is generated. |
CC @neild |
I am not convinced that I think there's a good argument that I am dubious, however, about the concern that returning a |
I think that not returning a 403 for ErrPermission would already be a win. That said, if an attacker can probe the filesystem and find out which files exist based on the server returning 404 or 500 this would still be an issue, but we might decide to accept the risk for the sake of usability. |
ServeFile
andFileServer
are used to serve static files.These files have to exists on the server or the two functions will return a 404.
The issue is that both function don't just return 404 by default, but they try to return an error that gives information on the server's local filesystem:
go/src/net/http/fs.go
Lines 597 to 610 in 3075ffc
This is the current implementation of
toHTTPError
:go/src/net/http/fs.go
Lines 672 to 681 in 3075ffc
This means that an attacker can probe the server's filesystem and discover potentially existing files that the server can't access, or otherwise infer information on the current directory tree (even if those paths are not accessible by the user running the server).
I propose we change this behavior to make all errors returned by
serveFile
be a 404.Note that there already are security-sensitive implementations out there trying to get around this kind of behavior. It would be nice if people didn't have to wrap the response writer in order to avoid introducing OWASP TESTING GUIDE-4.8.1: Improper error handling in their code.
/cc @FiloSottile @katiehockman @rolandshoemaker @bradfitz @neild
The text was updated successfully, but these errors were encountered: