Static module: get prefix from fixed params explicitly

[ghost]

We have discussed it with @pushrax. I'll add additional details to this PR a bit later.

[sourcegraphbot]

 (controllers.Static).Serve(prefix string, filepath string) github.com/revel/revel.Result

 (controllers.Static).ServeModule(moduleName string, prefix string, filepath string) github.com/revel/revel.Result

---

View the full smart diff on ✱ sourcegraph.com

Disable sourcegraphbot for this repository.

[ghost]

Who controls @revelframework twitter account? Can v0.11.2 bugfix release be announced? Those who use revel in production are recommended to upgrade it ASAP. From a dozen of projects only two are not affected by the bug.

[brendensoares]

@AnonX @robfig controls the twitter account. I DM'd him about the new hotfix.

[landaire]

How did this cause a vulnerability? I thought that #503 was fixed a long time ago.

[ghost]

@brendensoares, 👍
@landaire, I think additional info should not be shared publicly until projects in the wild fix the vuln. So, I emailed you the details.

[notzippy]

This fix broke the Static.ServeModule for my included modules. It is using a relative path from my main applications public folder and not the path of the module folder. My modules contain public folders and were included in the routes like
/public/modulefoo/*filepath StaticServer("modulefoo","public")

The directory structure was
modulefoo/app
modulefoo/public

With this change it looks for the file in the mainapp folder which is unexpected.
mainapp/public/modulefoo/

[ghost]

@notzippy, unfortunately you are right. I didn't pay attention to the fact that ServeModule calls Serve rather than returning result on its own. And Serve does use value from Fixed params now instead of the input argument. Thus, working version should look like:
https://gist.github.com/anonx/4faf012749485965fe8d
Are you using revel in production?

[notzippy]

@AnonX Looking for dec 1 deployment, will be testing in next few days in the wild though

[ghost]

@notzippy Then there are a few options:

1. One more release (v0.11.3) with the fix in gist or changes to master without new release
2. You can clone static module, apply the changes from gist and replace module.static=...'s value in your conf/app.conf. So, use your own static module.
3. You can use v0.11.1 (before patch) but remove wildcard route from your conf/routes file, so it wouldn't be affected by the bug.

I personaly use the 4th option:
4. Do not use static module in production, let nginx care about your static files. It is not always possible though.

[notzippy]

I can make the changes to file itself, but the issue may be else were, currently Serve is being called instead of ServeModule for the module in question.. I am trying to track this down. Do you have access to the irc channel rather then filing this log :-) ? If not that is fine I can continue back tracing to see were things are going wrong. Currently the router appears to be creating the route properly...

[notzippy]

@AnonX looking at the road map, v.12 is going to be a while yet - I think we should release the change to fix static includes sooner as opposed to later. @brendensoares what are your thoughts ?

[brendensoares]

I've been out of the loop for a bit too long. I'm not sure what the issue is here, though I trust it's a real issue.

Could I have some steps to reproduce this along with the expected results versus the actual results? That way I can better contribute to the conversation.

I have to catch up on a lot of other issues, so I'll watch for this thread. Thanks.

[ghost]

@brendensoares http://localhost:9000/Static/Serve?prefix=/etc&filepath=passwd or prefix=conf&filepath=app.conf. This worked on any project that had a wildcard route.

[brendensoares]

@AnonX I read through the related issues and now I see what's happening. Hope most people are using nginx or the like 😞