[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:
duplicate
relation
is related to MGNLRES-281 FTL and YAML files are exposed over t... Closed
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: Nucleus

 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
https://demo.magnolia-cms.com/.resources/NOTICE.txt
https://demo.magnolia-cms.com/.resources/logging.properties
https://demo.magnolia-cms.com/.resources/ErrorProcess.bpmn2
https://demo.magnolia-cms.com/.resources/PropertyNames.txt
https://demo.magnolia-cms.com/.resources/log4j.xml
https://demo.magnolia-cms.com/.resources/org/apache/catalina/startup/catalina.properties
etc, etc...

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.
2) consider disabling the classpath resource access entirely, or base it on a subfolder concept like it used to be. If you leave it, configuration should disable access by default except where specifically allowed.
3) add the JCR permission layer back in for JCR resources. It does not make sense that you set permissions for resources workspace in JCR, which then get ignored when resources are accessed.

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 MGNLRES-281 (comments already shifted in this direction over there).

[In my opinion], neither blacklisting nor whitelisting is a silver bullet for exposing/hiding resource types—we'll never list all potential types one may use [...], it could rather be circumvented by design [like mgnl-resources used to cater to].
Anyway, it's nothing we'd change in a haste at all, current situation is generally ok, while it still is possible to do more fine-grained URI security.

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?
Then do I get this right that they're used for both hiding selected sensitive files, as well as mapping renderers to file patterns?
I'd gladly figure out if we can advise something there, at least for the exposure part—even if it works as intended (cc mmuehlebach), I agree it can be regarded as a flaw of the current design.

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.
Alternatively, I've been advocating for a while for the idea to configure pre/post-processors as filters on that resources servlet eventually, which would be somewhat more composable. It's a bit blind-sighted without prototyping it first, let's see if we can arrange that in the next few weeks.

Cheers,
Mika

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:

  • Basically, the resources module includes a new config, "ResourceRendererDefinitions". Each defintion ties a renderer class to a voter.
  • When it is time to render a resource, the resource servlet processes the list of definitions in order, testing each voter against the resource object.
  • the first voter voting yes wins, and the associated renderer is used to render the resource.
  • if no voters vote yes, a default renderer is used (the Error403Renderer in our setup)

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.
If this is really as intended, then you should have put a huge, red-letter warning: "WARNING! Installing this upgrade will expose your entire classpath to the internet and disable your JCR permissions in the resources workspace..."
But my belief is that this was not really intended, and simply "just happended" due to a lack of prior analysis...

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?
What should we mention on the Release notes?

Comment by Martin Drápela [ 12/Oct/16 ]

Only a very non-specific note has been added to 5.5 Release notes.

Generated at Mon Feb 12 06:49:12 CET 2024 using Jira 9.4.2#940002-sha1:46d1a51de284217efdcb32434eab47a99af2938b.