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

[4.0] Expand the allowed list for the HTML sanitiser #34518

Merged
merged 3 commits into from
Jun 29, 2021

Conversation

dgrammatiko
Copy link
Contributor

Pull Request for Issue # .

Summary of Changes

By default, the sanitiser will remove all the form elements. This PR changes this behaviour so form elements would not disappear (on* events will still be removed).
The list of attributes comes directly from the MDN: https://developer.mozilla.org/en-US/docs/Web/HTML/Element/input#attributes

Testing Instructions

Apply the PR
Goto admin dashboard and open the browser console
pass some HTML to the sanitiser and check the result:

Joomla.sanitizeHtml(`<input type="text" onchange="alert('nope')" >`);

Check more attributes and elements (button, select, textarea)

Actual result BEFORE applying this Pull Request

Too strict

Expected result AFTER applying this Pull Request

Less strict

Documentation Changes Required

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-4.0-dev labels Jun 14, 2021
@RickR2H
Copy link
Member

RickR2H commented Jun 23, 2021

@dgrammatiko I applied the patch and added to the console: Joomla.sanitizeHtml(<input type="text" onchange="alert('nope')" >);
Got the following in return: "<input type="text">"

Is this correct?


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

@dgrammatiko
Copy link
Contributor Author

Is this correct?

Yes, the onchange="alert('nope')" is Javascript so correctly was removed by the sanitizer

@RickR2H
Copy link
Member

RickR2H commented Jun 25, 2021

I have tested this item ✅ successfully on 1fe402d


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

1 similar comment
@jwaisner
Copy link
Member

I have tested this item ✅ successfully on 1fe402d


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

@jwaisner
Copy link
Member

RTC


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

@joomla-cms-bot joomla-cms-bot added the RTC This Pull Request is Ready To Commit label Jun 29, 2021
@wilsonge wilsonge merged commit 49d6b71 into joomla:4.0-dev Jun 29, 2021
@wilsonge
Copy link
Contributor

Thanks!

@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Jun 29, 2021
@wilsonge wilsonge added this to the Joomla 4.0 milestone Jun 29, 2021
@dgrammatiko dgrammatiko deleted the patch-2 branch April 18, 2022 09:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NPM Resource Changed This Pull Request can't be tested by Patchtester
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants