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

Try to avoid triggering modsec rules #5971

Merged
merged 1 commit into from Apr 6, 2018

Conversation

Projects
None yet
2 participants
@pento
Member

pento commented Apr 4, 2018

Description

Since Gutenberg 2.3 (probably #5253), API requests have been triggering modsec rules.

Unfortunately, there isn't much to go on, so this PR is for trying out some techniques to potentially fix it.

See #5867.

@pento pento self-assigned this Apr 4, 2018

@pento

This comment has been minimized.

Member

pento commented Apr 4, 2018

@SuperGeniusZeb: I've tried comparing the raw POST requests that Gutenberg 2.2 and 2.5 made. The most obvious difference is that the former sent data as JSON, whereas the latter users form encoding. Depending on how modsec parses the POST data, this may effect whether a rule is being triggered or not.

Could you test this change, and see if it fixes the problem, or maybe causes a different rule to be triggered?

@ZebulanStanphill

This comment has been minimized.

Contributor

ZebulanStanphill commented Apr 5, 2018

I have not tested the change yet, but I did notice a mistake in your pull request. You need to change the closing identifier on lines 113 and 130 of /lib/compat.php ($api_request_fix = <<<JS) so that options.data = JSON.stringify( options.data ); doesn't prematurely end the heredoc multiline string, as it contains the closing identifier "JS".

@ZebulanStanphill

This comment has been minimized.

Contributor

ZebulanStanphill commented Apr 5, 2018

I fixed the heredoc thing on my own copy of the Gutenberg plugin and manually applied your patch... and it works! I can now save and publish posts containing SQL phrases. 😄

@pento

This comment has been minimized.

Member

pento commented Apr 6, 2018

@SuperGeniusZeb: Awesome news!

I'm curious, what version of PHP are you using that's causing the heredoc to be closed because "JSON" starts with "JS"? Per the PHP docs, the heredoc should only be closed by the line that only contains "JS;".

@ZebulanStanphill

This comment has been minimized.

Contributor

ZebulanStanphill commented Apr 6, 2018

@pento Oh. I thought it would close there because it appeared to according to the syntax highlighting from CodeMirror on the plugin editor in WordPress.

image

I guess that indicates a bug in the syntax highlighting?

@pento

This comment has been minimized.

Member

pento commented Apr 6, 2018

Yup, that's a bug in CodeMirror by the look of it. I was able to reproduce it on their demo page, so I've submitted a bug report. 🙂

@pento pento added this to the 2.7 milestone Apr 6, 2018

@pento pento merged commit 4e7a0c7 into master Apr 6, 2018

2 checks passed

codecov/project 44.34% remains the same compared to ea9416b
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@pento pento deleted the fix/5867-blocked-requests branch Apr 6, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment