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

Fix #46244 Remove innerHTML usage to avoid self-XSS #46269

Merged
merged 1 commit into from Oct 22, 2022

Conversation

codergeek121
Copy link
Contributor

@codergeek121 codergeek121 commented Oct 18, 2022

Motivation / Background

This Pull Request has been created because of #46244 . Fixes #46244

Detail

I replaced innerHTML usages with innerText. Problematic input is now displayed and not evaluated.
To test it, use the suggested #46244 (comment) input. There should be no alert box and the displayed <tr>s are not broken but display the input.

@rails-bot rails-bot bot added the actionpack label Oct 18, 2022
@ghiculescu
Copy link
Member

This PR looks good. The red tests seem unrelated, but you could rebase to main to re-run them and confirm that. Could you add "Fixes #46244" to the PR body, then it will close the issue automatically when it's merged.

@ghiculescu ghiculescu added the ready PRs ready to merge label Oct 20, 2022
@codergeek121
Copy link
Contributor Author

Thank you for the review. I rebased and everything is passing now :)

@byroot byroot merged commit 87cf97b into rails:main Oct 22, 2022
byroot added a commit that referenced this pull request Nov 1, 2022
byroot added a commit that referenced this pull request Nov 1, 2022
byroot added a commit that referenced this pull request Nov 1, 2022
@pboling
Copy link
Contributor

pboling commented Jan 31, 2023

@byroot This fix did not make it into any change log, nor any release notes (that I can find), and Synk is still reporting it as an unfixed, current vulnerability in 7.0.4.2, with no possible upgrade path to any fixed version. It also says that a fix has been merged to master, but remains unreleased.

Screenshot 2023-01-31 at 12 07 56 PM

Of course the fix has been released.

How can we avoid this situation in the future?

I have reported the error in Synk's vulnerability database to Synk.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
actionpack ready PRs ready to merge
Projects
None yet
Development

Successfully merging this pull request may close these issues.

¬ XSS within Route Error Page