Skip to content
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

Joomla 4 > Modals in the backend throwing a 403 or 404 error because of ModSecurity #38404

Closed
woluweb opened this issue Aug 5, 2022 · 11 comments

Comments

@woluweb
Copy link
Contributor

woluweb commented Aug 5, 2022

Steps to reproduce the issue

This issue relates to 3 Modals in Joomla 4 backend:

  1. when you Edit an Article from a Menu Item (not when you create it)
  2. when you Edit the Association between Articles in Multilingual sites
  3. when you Edit the Association between Menu Items in Multilingual sites

I had noticed this a few months ago but could not find anybody else facing the same issue.
Then @dgrammatiko found this workaround:
https://joomla.stackexchange.com/a/32273/15734

But still I only had an issue on 1 site and did not know what the root cause was.
A few days ago, I had the issue on a 2nd site... and yesterday somebody posted on FB that she had the same issue:
https://www.facebook.com/groups/joomlanospam/posts/10158437621590997/
which explaind the "root cause", being a rule in ModSecurity (and probably not all hosts have the same rules... and in the case of Shared Hosting users can't change the rules I guess: only disable/enable ModSecurity)

So now that I have the fix and the explanation, I take the time to post it here.

In each of the 2 following files, only one line of code has to be fixed in order to avoid this issue:

  • /administrator/components/com_content/src/Field/Modal/ArticleField.php
  • /administrator/components/com_menus/src/Field/Modal/MenuField.php

More specifically, this bit
.edit&id=\' + document.getElementById("' . $this->id . '_id").value + \'';
should be replaced by
.edit&id=+ document.getElementById("' . $this->id . '_id").value +';
in order to avoid being blocked by ModSecurity

[ note that Dimitris will definitely be right suggesting that those 2 files should be completely rewritten. But in the meantime, if those 2 lines could be fixed for the next release in August, it would already avoid having users being blocked ]

Expected result

Modals do work :)

Actual result

System information (as much as possible)

Additional comments

@brianteeman
Copy link
Contributor

Please test #38628

@dgrammatiko
Copy link
Contributor

Brian that doesn’t solve the xss issue…

@brianteeman
Copy link
Contributor

I was just creating the pr with the code suggested. Nothing more

@woluweb
Copy link
Contributor Author

woluweb commented Aug 30, 2022

Txs @brianteeman
As you could see in you PR, I tested it successfully :)

@dgrammatiko
Indeed that does not solve the xss issue, but that is another topic requiring I guess a more general rewrite of different files.
So at least in the meantime we have solved this error affecting certain websites.

@richard67
Copy link
Member

Closing as having a pull request which fixes the reported issue. For anything else I suggest to open a new issue if the isn't one already.

@dgrammatiko
Copy link
Contributor

For anything else I suggest to open a new issue if the isn't one already.

XSS shouldn't actually have an issue as they are SECURITY ISSUES and there's another process for those. The fact that it is now publicly exposed makes the proposed patch insufficient and the actual vulnerability needs to be addressed.

My 2c

@laoneo laoneo reopened this Nov 28, 2022
@laoneo
Copy link
Member

laoneo commented Nov 28, 2022

Reopening this as the pr was not accepted.

@dgrammatiko
Copy link
Contributor

I've took the code code from the old PR by @okonomiyaki3000 that was supposed to fix the modals (or at least improve them) and patched it for 4.3 here. It trades one XSS with another one BUT this could be tackled easily, so, I need someone from the maintainers group to check if there's a possible B/C break before spending any time there.

@HLeithner @laoneo

@okonomiyaki3000
Copy link
Contributor

👍 I probably won't be involved much in Joomla going forward but it's great to see this fix lives on. I don't remember much about it TBH but still...

@dgrammatiko
Copy link
Contributor

Hey @okonomiyaki3000 nice to see you around. I want to apologise that your PR didn't make on 4.0 somehow it was a miscommunication problem between me and @wilsonge. Anyways if the maintainers give the new PR the green light regarding B/C I will redo it so your name will show up in the history as it should be (it's your code), my intention is not to steel your code.

@brianteeman
Copy link
Contributor

as the modal have been rewritten I suspect that this is no longer an issue

@Quy Quy closed this as completed Mar 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

9 participants