NPM Resource Changed PR-5.2-dev Pending

User tests: Successful: Unsuccessful:

avatar Shivam7-1
Shivam7-1
31 Oct 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

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

avatar Shivam7-1 Shivam7-1 - open - 31 Oct 2024
avatar Shivam7-1 Shivam7-1 - change - 31 Oct 2024
Status New Pending
avatar joomla-cms-bot joomla-cms-bot - change - 31 Oct 2024
Category JavaScript Repository NPM Change
avatar Shivam7-1 Shivam7-1 - change - 31 Oct 2024
The description was changed
avatar Shivam7-1 Shivam7-1 - edited - 31 Oct 2024
avatar Shivam7-1 Shivam7-1 - change - 4 Nov 2024
Labels Added: NPM Resource Changed PR-5.2-dev
avatar Shivam7-1 Shivam7-1 - change - 7 Nov 2024
The description was changed
avatar Shivam7-1 Shivam7-1 - edited - 7 Nov 2024
avatar Shivam7-1
Shivam7-1 - comment - 7 Nov 2024

Hii @Hackwar this PR is ready for review.
Could you please take a look when you have time? Let me know if you have any questions or suggestions. Thanks!

avatar laoneo
laoneo - comment - 7 Nov 2024

How this can actually being exploited within the core? Can you provide some testing instructions how we can reproduce the issue you are trying to fix?

avatar Shivam7-1
Shivam7-1 - comment - 7 Nov 2024

Hii @laoneo Thanks For Review
Here is Explanation

Explanation of Fix:

The change implemented here sanitizes the HTML content generated by qr.createImgTag(4) before it is inserted into the DOM.

Before the Change:

elTarget.innerHTML = qr.createImgTag(4);

This approach directly inserts the raw HTML output from qr.createImgTag(4) into the DOM. If this function can be influenced by untrusted input (e.g., user input or external data), it could potentially introduce security vulnerabilities such as Cross-Site Scripting (XSS). This would allow an attacker to inject malicious JavaScript (e.g., via <script> tags or event handlers) into the page, leading to the execution of arbitrary scripts in the user's browser. For example, an attacker could steal cookies, session tokens, or perform actions on behalf of the user.

After the Change:

elTarget.innerHTML = Joomla.sanitizeHtml(qr.createImgTag(4));

By using Joomla.sanitizeHtml(), we ensure that any potentially dangerous content (like <script> tags, event handlers, or inline JavaScript) is stripped from the generated HTML. This significantly reduces the risk of XSS attacks and ensures the content inserted into the page is safe.

Thank

avatar laoneo
laoneo - comment - 7 Nov 2024

But we need some testing instructions to actually see what for a bug it fixes.

avatar brianteeman brianteeman - test_item - 7 Nov 2024 - Tested successfully
avatar brianteeman
brianteeman - comment - 7 Nov 2024

I have tested this item ✅ successfully on e57055f

code review


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

avatar Shivam7-1
Shivam7-1 - comment - 7 Nov 2024

Hii @brianteeman
Is there Anything Else is Required From My Side?
Thanks

avatar SniperSister
SniperSister - comment - 8 Nov 2024

The actual markup for innerHTML is not userprovided, there is no vector for an XSS here.

I suggest to close the PR.

Nevertheless: thank you!

avatar SniperSister SniperSister - change - 8 Nov 2024
Status Pending Closed
Closed_Date 0000-00-00 00:00:00 2024-11-08 07:25:07
Closed_By SniperSister
avatar SniperSister SniperSister - close - 8 Nov 2024

Add a Comment

Login with GitHub to post a comment