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

Fix too much data being sent along to the package install request #13813

Merged
merged 1 commit into from Mar 26, 2018

Conversation

Projects
2 participants
@Mark-H
Collaborator

Mark-H commented Mar 12, 2018

What does it do?

In the (ExtJS) package install handler, the va variable is the event being fed into the install method. This is a click event on the "Continue" button. This data was then being added on with the signature and registry information for the console for the actual installation to be done server-side.

All the data in va is unnecessary for the installation. It contains raw javascript code (the handler code on the button) and all sorts of other event data. I've removed the va variable from being used here.

Why is it needed?

Aside from sending way too much data, some of this data is likely to trigger firewalls/mod_security type WAFs. This would prevent people from installing packages in such environments.

How to test?

Keep the browser dev tools open when starting a package installation/reinstallation/update. Note the first request to connector.php at this point. Prior to the patch, it sends lots of javascript data in the request parameters. After the patch, only the information needed to do the install is sent.

Related issue(s)/PR(s)

modmore support ticket 16334, related forum thread https://forums.modx.com/thread/103572/extras-fail-to-update-install#dis-post-557258

@Mark-H Mark-H added this to the v2.6.2 milestone Mar 12, 2018

@gpsietzema gpsietzema added this to Testing in MODX3 Mar 20, 2018

@opengeek opengeek merged commit 8e65a8d into modxcms:2.6.x Mar 26, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@gpsietzema gpsietzema moved this from Testing to Done in MODX3 Apr 9, 2018

Mark-H added a commit to Mark-H/revolution that referenced this pull request Apr 11, 2018

Fix installation of transport packages with setup options
In modxcms#13813 I removed the va variable from being sent to the processor as that contained an extjs config object which was tripping over mod_security rules. But I failed to notice that setup options from a package were also being sent in this variable, so in 2.6.2 setup options were no longer being sent to the server, breaking installation (or preventing easy set-up) of some packages.

To remove the ambiguity of the va variable I've updated the setup options install method to send the same variables to install, with the options as a new third parameter instead. That way it can be treated properly. I've also added extensive comments for the next poor sap to explore into the depths of the package installation process.

raffy99 added a commit to raffy99/revolution that referenced this pull request Nov 9, 2018

Fix installation of transport packages with setup options
In modxcms#13813 I removed the va variable from being sent to the processor as that contained an extjs config object which was tripping over mod_security rules. But I failed to notice that setup options from a package were also being sent in this variable, so in 2.6.2 setup options were no longer being sent to the server, breaking installation (or preventing easy set-up) of some packages.

To remove the ambiguity of the va variable I've updated the setup options install method to send the same variables to install, with the options as a new third parameter instead. That way it can be treated properly. I've also added extensive comments for the next poor sap to explore into the depths of the package installation process.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment