Why no responsible disclosure
First off, I wanted to proceed with a responsible disclosure but:
- I didn't find a security policy for this repo
- The only contact info I found was a Twitter account that was restricted to only followers so I could not send a DM or tweet
The vulnerability
|
req.Params["SCRIPT_FILENAME"] = path.Join(fs.DocRoot, urlPath) |
This line takes remote user controllable path from HTTP request and appends it via filepath.Join() (it does relative-to-absolute translation) to a file path, now making it a legit file path from now on.
Now, given user code:
handler := gofast.NewHandler(
gofast.NewPHPFS("/baikal/html/"),
gofast.SimpleClientFactory(gofast.SimpleConnFactory("tcp", ":8080"), 0),
)And an attacker sending a HTTP request:
$ curl --path-as-is http://localhost/../../etc/passwd
(curl would normalize the path without --path-as-is)
Because the handler also serves static files, I am able to steal anything from the filesystem (even ENV vars from /proc/self/environ that sometimes are used to store secrets).
If I add logging, the above request would show up as:
incoming URL path=/../../etc/passwd SCRIPT_FILENAME=/etc/passwd
More on this vulnerability: https://owasp.org/www-community/attacks/Path_Traversal
Other observations
Ineffectual code
|
req.Params["SCRIPT_FILENAME"] = filepath.Join(fs.DocRoot, fastcgiScriptName) |
This is ineffectual, as req.Params["SCRIPT_FILENAME"] is overwritten a few lines later
Performance observation
|
pathinfoRe := regexp.MustCompile(`^(.+\.php)(/?.+)$`) |
This compiles a regex on every request. It might be more performant to compile the regex only once (outside of this function). There's another place also that compiles a regex on every request.
Why no responsible disclosure
First off, I wanted to proceed with a responsible disclosure but:
The vulnerability
gofast/session.go
Line 227 in e5eda92
This line takes remote user controllable path from HTTP request and appends it via
filepath.Join()(it does relative-to-absolute translation) to a file path, now making it a legit file path from now on.Now, given user code:
And an attacker sending a HTTP request:
$ curl --path-as-is http://localhost/../../etc/passwd(curl would normalize the path without
--path-as-is)Because the handler also serves static files, I am able to steal anything from the filesystem (even ENV vars from
/proc/self/environthat sometimes are used to store secrets).If I add logging, the above request would show up as:
More on this vulnerability: https://owasp.org/www-community/attacks/Path_Traversal
Other observations
Ineffectual code
gofast/session.go
Line 218 in e5eda92
This is ineffectual, as
req.Params["SCRIPT_FILENAME"]is overwritten a few lines laterPerformance observation
gofast/session.go
Line 210 in e5eda92
This compiles a regex on every request. It might be more performant to compile the regex only once (outside of this function). There's another place also that compiles a regex on every request.