-
-
Notifications
You must be signed in to change notification settings - Fork 3.6k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Prevent serialization of the JApplication object #16587
Comments
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
This comment was marked as abuse.
Too much of the application is transient I think to sanely deal with that:
I think we're looking at a case where this object, by design, shouldn't be serialized and stored somewhere as a whole, and definitely shouldn't be unserialized and re-used. |
Regarding __sleep(), yes, it is possible to use it correctly but much
more complicated (I had already considered that). The problem is that
__sleep() is expected to return an array with the properties which
should be serialized (as opposed to those which shouldn't). This means
we have to go through reflection and apply a blacklist. The blacklist
can be defined in a protected property, allowing us to override it in
descendant classes. Not pretty, but works.
My original proposal was on the grounds that JApplication is not meant
to be serialized. We should make it abundantly clear to developers.
Using magic methods to prevent serialization or direct instantiation is
uncommon but not forbidden. Before abstract classes we'd use a
constructor that threw an error to prevent instantiation of helper
classes, for example.
Anyway, either solution works for me. All I care about is that people's
sites are secure.
|
Unless there is a strong argument for keeping support, I think this is one class chain where we need to just full stop block serialization. And in general it'd be a good time to review high level parts of the API to sort out where we need to start defining serializable aspects of objects a bit better (i.e. K2 doing something where it serializes a |
Well J4 would be a good opportunity to block the serialization - not that I have a clue how |
Closing since nobody cares to do anything about it for two years. |
The problem
The last few weeks I have come across at least one template provider whose templates, due to negligence and/or lack of understanding of how Joomla! works, tries to store a reference to the JApplication object in an object that ends up in the Joomla! cache. As you know, this means that the object gets serialized on cache save and resumed (woken up) on cache read.
That wouldn't be all that bad except for the fact that JApplication holds transient information such as the JInput object (GET, POST and FILES request data) and the plugins event dispatcher. This opens a security hole since a plugin could now be cached under a Super User and resumed under a plain user. This could easily and plausibly be the case of a content plugin, thus leading to information disclosure.
In the unlikely event the site owner is so profoundly lucky that they do not get a security issue, they will most likely get quirky behavior. Even worse, they are likely to blame the plugin developer -not the offending template developer- for the misbehaving plugins. This is understandable. From the user's point of view there's no way to connect the odd behavior with the template.
Proposed solution
JApplication should have a magic
__sleep
method like this:Perhaps we could add "Go to your site's administrator backend, System, Global Configuration and set Caching to Off to fix this issue". We could even make this string localizable. Even better, we could like to a Joomla! documentation page explaining the security and behavior aspects of this bug in third party software.
Even though in the short term this could be moderately annoying for the users, in the medium term we are improving the security and reliability of thousands of Joomla! sites and forcing template developers to adopt the same sane security practices that we, core and backend developers, have been following and taking for granted for over a decade.
Tagging @mbabker and @wilsonge since they are qualified to discuss this and we have spoken briefly about it at JaB17. Tagging @PhilETaylor since he knows his security stuff ;) Finally, tagging @rdeutz since he's the release manager of the current branch. Apologies if I forgot someone else, feel free to chip in your opinion.
The text was updated successfully, but these errors were encountered: