Skip to content

[6.1] Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML#44424

Open
Shivam7-1 wants to merge 3 commits intojoomla:6.1-devfrom
Shivam7-1:patch-5
Open

[6.1] Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML#44424
Shivam7-1 wants to merge 3 commits intojoomla:6.1-devfrom
Shivam7-1:patch-5

Conversation

@Shivam7-1
Copy link
Contributor

@Shivam7-1 Shivam7-1 commented Nov 8, 2024

Summary of Changes

In This PR Joomla.sanitizeHtml to sanitize all HTML content rendered within the application. This change improves security by preventing XSS (Cross-Site Scripting) vulnerabilities and ensures that user-generated or external HTML is safe. All relevant components have been updated for consistent sanitization, enhancing overall application integrity.

Link to documentations

Please select:

  • Documentation link for docs.joomla.org:

  • No documentation changes for docs.joomla.org needed

  • Pull Request link for manual.joomla.org:

  • No documentation changes for manual.joomla.org needed

@joomla-cms-bot joomla-cms-bot added NPM Resource Changed This Pull Request can't be tested by Patchtester PR-5.2-dev labels Nov 8, 2024
@richard67 richard67 added Test instructions missing Updates Requested Indicates that this pull request needs an update from the author and should not be tested. labels Nov 10, 2024
…6.js

Co-authored-by: Dimitris Grammatikogiannis <dg@dgrammatiko.dev>
@Shivam7-1
Copy link
Contributor Author

Shivam7-1 commented Nov 11, 2024

Hii Reviewers @dgrammatiko
Thank You so Much For Reviewing 😃

I understand the concern about using Joomla.sanitizeHtml() without proper configuration. The intent was to sanitize potentially unsafe HTML, but I agree that it could break things when not properly configured for specific elements and their attributes. As a result, I will update the code to use textContent, which will ensure that any HTML is rendered as plain text, avoiding the potential for broken content or issues with element attributes.

I don’t have a specific test case to demonstrate an exploit, but I can explain how the issue could be tested. The potential vulnerability lies in cases where user-provided content—such as input from forms or comments—could be injected into the page and rendered without proper sanitization.

I will proceed with the change to textContent to eliminate this risk

@Hackwar
Copy link
Member

Hackwar commented Nov 28, 2024

Thank you for creating this PR, however I'm not considering this a bugfix and at this point in time I will only accept bugfixes for 5.2. Please change the PR to be against 5.3-dev. Thank you.

@Shivam7-1 Shivam7-1 changed the base branch from 5.2-dev to 5.3-dev November 28, 2024 14:37
@Shivam7-1
Copy link
Contributor Author

Hii @Hackwar Thanks for Reviewing PR I had Changed this to 5.3-dev
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @Hackwar Thanks for Reviewing PR I had Changed this to 5.3-dev
Could You Please Review This PR again
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @Hackwar @dgrammatiko Thanks for Reviewing PR I had Done Changes According to Suggestions Could Team Review This PR again
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @Hackwar @dgrammatiko Thanks for Reviewing PR I had Done Changes According to Suggestions Could Team Review This PR again
Thanks

@Shivam7-1
Copy link
Contributor Author

Ping @dgrammatiko

@HLeithner HLeithner changed the base branch from 5.3-dev to 6.0-dev March 4, 2025 17:19
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 6.0-dev.

@HLeithner HLeithner changed the title Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML [6.0] Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML Mar 4, 2025
@Shivam7-1
Copy link
Contributor Author

Hii @HLeithner Could you llease review this PR
Thanks

@HLeithner
Copy link
Member

Someone with better javascript skills then me should do this, the last pr I merged by you, I had to create my own pr to fix it. So better some of the js experts should check this.

@rdeutz rdeutz removed the PR-5.3-dev label Mar 5, 2025
@Shivam7-1
Copy link
Contributor Author

Hii @HLeithner Thanks For your response could you ping anyone who is expert in this
Thanks

@Shivam7-1
Copy link
Contributor Author

Hii @rdeutz Could you please review this PR
Thanks

@HLeithner HLeithner changed the base branch from 6.0-dev to 6.1-dev August 31, 2025 11:58
@HLeithner
Copy link
Member

This pull request has been automatically rebased to 6.1-dev.

@HLeithner HLeithner changed the title [6.0] Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML [6.1] Update admin-update-default.es6.js SanitizHTML DOM Text Interpretd as HTML Aug 31, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Feature NPM Resource Changed This Pull Request can't be tested by Patchtester PR-6.0-dev PR-6.1-dev Test instructions missing Updates Requested Indicates that this pull request needs an update from the author and should not be tested.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants