Serious security vulnerability: path traversal

[joonas-fi]

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

https://github.com/yookoala/gofast/blob/e5eda92ffb0e1f2aeada38c3174c0d1d9adeb2da/session.go#L227

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

https://github.com/yookoala/gofast/blob/e5eda92ffb0e1f2aeada38c3174c0d1d9adeb2da/session.go#L218

This is ineffectual, as req.Params["SCRIPT_FILENAME"] is overwritten a few lines later

Performance observation

https://github.com/yookoala/gofast/blob/e5eda92ffb0e1f2aeada38c3174c0d1d9adeb2da/session.go#L210

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.

[yookoala]

Will fix it in this weekend. Thanks for reporting.

[yookoala]

@joonas-fi: both issues are addressed. Please see #61 and #62.

[yookoala]

All changes are published in v0.5.0.

[joonas-fi]

Thanks for fast response!

I'm terribly sorry I didn't communicate myself well, but the place I linked to was only one of the places where the HTTP request path is directly used as a FS path. There's another place here: https://github.com/yookoala/gofast/blob/b30b52d0e86df28db57c447fb749ac97eb8c3529/session.go#L348

Disclaimer: I'm not saying this second place is the last place that has a vulnerability. I found these two by quickly looking, but if there's more places that take filesystem paths from HTTP request those should be investigated as well. I'll try looking.