? ? Pending

User tests: Successful: Unsuccessful:

avatar brianteeman
brianteeman
29 Aug 2022

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.

avatar brianteeman brianteeman - open - 29 Aug 2022
avatar brianteeman brianteeman - change - 29 Aug 2022
Status New Pending
avatar brianteeman brianteeman - change - 29 Aug 2022
Labels Added: ?
avatar joomla-cms-bot joomla-cms-bot - change - 29 Aug 2022
Category Administration com_categories com_contact com_content com_menus com_newsfeeds
avatar brianteeman brianteeman - change - 29 Aug 2022
The description was changed
avatar brianteeman brianteeman - edited - 29 Aug 2022
avatar Abernyte-Git
Abernyte-Git - comment - 30 Aug 2022

I have tested this item successfully on 8464a13

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.

avatar Abernyte-Git Abernyte-Git - test_item - 30 Aug 2022 - Tested successfully
avatar woluweb
woluweb - comment - 30 Aug 2022

I have tested this item successfully on 8464a13

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.

avatar woluweb woluweb - test_item - 30 Aug 2022 - Tested successfully
avatar richard67 richard67 - change - 30 Aug 2022
Status Pending Ready to Commit
avatar richard67
richard67 - comment - 30 Aug 2022

RTC


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

avatar fancyFranci
fancyFranci - comment - 15 Sep 2022

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 :/

avatar dgrammatiko
dgrammatiko - comment - 17 Sep 2022

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

avatar laoneo
laoneo - comment - 19 Sep 2022

@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.

avatar dgrammatiko
dgrammatiko - comment - 21 Sep 2022

@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…

avatar laoneo
laoneo - comment - 28 Nov 2022

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.

avatar laoneo laoneo - change - 28 Nov 2022
Status Ready to Commit Closed
Closed_Date 0000-00-00 00:00:00 2022-11-28 08:24:34
Closed_By laoneo
Labels Added: ?
avatar laoneo laoneo - close - 28 Nov 2022
avatar brianteeman
brianteeman - comment - 28 Nov 2022

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

Add a Comment

Login with GitHub to post a comment