joomla-cms icon indicating copy to clipboard operation
joomla-cms copied to clipboard

Modals in the backend throwing a 403 or 404 error because of ModSecurity

Open brianteeman opened this issue 3 years ago • 7 comments

PR for #38404

Please read the explanation and comments on the issue

I am unable to test this as I dont have access to a server with the faulty mod_security rule but as no one else looked like they would create the PR here it is.

Based on @woluweb report this should work and I found a few other places to add.

brianteeman avatar Aug 29 '22 09:08 brianteeman

I have tested this item :white_check_mark: successfully on 8464a13e5bca77fc29694cb5177204e890ded7bb

Lacking the appropriate modSecurity rule to replicate the issue I have applied the patch and confirm the changes by code review.


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38628.

Abernyte-Git avatar Aug 30 '22 08:08 Abernyte-Git

I have tested this item :white_check_mark: successfully on 8464a13e5bca77fc29694cb5177204e890ded7bb

Txs for the Pull Request.

I have a server havving that mod_security issue so I can test in real conditions :)

Before applying the patch, I had the following error message within the Modal (when editing an Article from a Menu Item for example): _ Forbidden You don't have permission to access this resource._

After applying the patch, the modal was working normally, showing the corresponding Article


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38628.

woluweb avatar Aug 30 '22 12:08 woluweb

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/38628.

richard67 avatar Aug 30 '22 12:08 richard67

Unfortunately this is not the right way to fix the issue, like Dimitris mentioned. I will be happy to merge a complete refactoring, but I don't want to hide the valid warning, sorry :/

fancyFranci avatar Sep 15 '22 21:09 fancyFranci

@fancyFranci please reconsider merging this PR as the complete refactoring probably can't happen in the 4.x

dgrammatiko avatar Sep 17 '22 19:09 dgrammatiko

@dgrammatiko can you shed some light into the change here as for me it looks like invalid JS code when you have something like

&task=category.edit&id=+ document.getElementById("' . $this->id . '_id").value +

I have to say that I didn't digg into the code which uses this variable, but from a first glance it looks not right, even it fixes the issue.

laoneo avatar Sep 19 '22 06:09 laoneo

@laoneo the js code is passed inside the urlparams. In order that code to be come executable there’s an eval() and there is the problem, in sort the id of the field should be passed as a string but then the whole form field modals needs a rewrite as it is based on this monstrosity…

dgrammatiko avatar Sep 21 '22 07:09 dgrammatiko

After several discussions in maintainers chat. It is better to close this one and correctly implement the issue, even when it requires a rewrite. I'm also reopening the issue. Thanks for understanding.

laoneo avatar Nov 28 '22 08:11 laoneo

Suprised as the person stating it is not perfect also states that it cant be fixed any other way

brianteeman avatar Nov 28 '22 08:11 brianteeman