[MGNLRES-284] Exposed files via new resources module Created: 17/Aug/16 Updated: 29/Mar/22 Resolved: 20/Sep/16 |
|
| Status: | Closed |
| Project: | Magnolia Resources Module |
| Component/s: | None |
| Affects Version/s: | 2.4.6 |
| Fix Version/s: | 2.4.8, 2.5 |
| Type: | Bug | Priority: | Critical |
| Reporter: | Richard Unger | Assignee: | Ilgun Ilgun |
| Resolution: | Fixed | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Environment: |
LFRZ |
||
| Issue Links: |
|
||||||||||||
| Template: |
|
||||||||||||
| Acceptance criteria: |
Empty
|
||||||||||||
| Task DoD: |
[ ]*
Doc/release notes changes? Comment present?
[ ]*
Downstream builds green?
[ ]*
Solution information and context easily available?
[ ]*
Tests
[ ]*
FixVersion filled and not yet released
[ ] 
Architecture Decision Record (ADR)
|
||||||||||||
| Bug DoR: |
[ ]*
Steps to reproduce, expected, and actual results filled
[ ]*
Affected version filled
|
||||||||||||
| Release notes required: |
Yes
|
||||||||||||
| Documentation update required: |
Yes
|
||||||||||||
| Date of First Response: | |||||||||||||
| Visible to: |
Bence Vass, Marcus Büttner, Matthias Müller, Rihard Monovic
|
||||||||||||
| Sprint: | Basel 62 | ||||||||||||
| Story Points: | 8 | ||||||||||||
| Team: | |||||||||||||
| Description |
|
Yet another problem with the new resources module, unfortunately: The new resources module exposes anything on the classpath that can be referenced by path. In this way it is possible to retrieve things like: https://demo.magnolia-cms.com/.resources/freemarker/version.properties Compare this to earlier versions, where the ClasspathSpool servlet only accessed stuff stored within "mgnl-resources" directories. This is a serious information disclosure security problem. Theoretically the classpath can contain passwords, secret configuration infos, etc... It seems to me that the new resources layer was designed without much regard to security. The JCR resources are also all loaded via system session. From our point of view, the following changes are needed: 1) change it from "serve everything" to a white-listing model, so that only specifically defined resources get served. Include definitions for common resource types like CSS, JS, Fonts and Images. Since we needed to solve this urgently in our production environment, I have created a patch for the resources module, which I will attach. The patch addresses both this issue (whitelisting) and the issue of processed resources. Please take a look at it and consider this type of approach to solve the many problems of the resources module. Thank you! |
| Comments |
| Comment by Mikaël Geljić [ 25/Aug/16 ] |
|
Yes, I know, pretty much same stance from my side as on linked
Meanwhile, runger may I ask where do you make use of those two voters? —do you turn off combinations of origin+path on each resource-renderer? Re: Resources rendering, nice to see how you're addressing it! As far as I could see, it's pretty much along the same lines as the old processed resources, just on that different endpoint. Cheers, |
| Comment by Richard Unger [ 29/Aug/16 ] |
|
Hi Mika, Thanks for getting back to me, and thanks for the feedback. I use the voters within the changes I made to the resources module. I tried to set it up in a "magnolia-like" way:
This approach solves both current problems of the resource module: 1) It solves the security problem: You can set up white or black-listing through the voter configuration. You can also set up an approach like the mgnl-resources folders in the classpath. It's just a matter of configuring the voters. 2) It solves the problem of processed resources: by having a rendering step when resources are spooled out, you can handle freemarker, and theoretically other "compiled resources" like SCSS. We're currently experimenting with the setup and finding the best voter configuration for us. Ideally we would like to take it back to serving only from "mgnl-resources" folders in the classpath. So yes, it is a matter of setting up the correct sequence of voters that allows some accesses, and forbids the ones we don't want based on a combination of Origin and Path. As for the resources rendering, I like the solution that any text resource could be freemarker. I think this is a nice feature, and marking and pushing back the stream a simple way to handle it. It is working well so far in our tests. As for working as intended, I cannot disagree with this more: you went from a situation where only stuff in mgnl-resources folders was exposed, to exposing everything on the classpath, and didn't even mention it in the release notes. You went from having working JCR permissions to serving all resources with superuser rights. In any case, we are eager to find a solution for this that we don't have to maintain as a branch to the standard product, so please do let us know if we can help to move this forward! Thanks and regards from Vienna, Richard |
| Comment by Christoph Meier [ 03/Oct/16 ] |
|
ilgun - please mention what has been changed. Is there new config? New Magnolia property? |
| Comment by Martin Drápela [ 12/Oct/16 ] |
|
Only a very non-specific note has been added to 5.5 Release notes. |