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

add removeXSS plugin #1664

Closed
wants to merge 1 commit into from
Closed

add removeXSS plugin #1664

wants to merge 1 commit into from

Conversation

cakeinpanic
Copy link

Hi everyone!

I'm using svgo in my project and missing this plugin, so I added it :)
It removes all possible XSS attacks: with event listeners or direct script injection

@cakeinpanic
Copy link
Author

fixing #1665

@nanderv
Copy link

nanderv commented May 26, 2023

Hi, will this feature still be added?

@pi0
Copy link

pi0 commented Oct 18, 2023

I have directly enabled this plugin in unjs/ipx (powring Nuxt image).

Thanks for working on this PR @cakeinpanic and wish you best 🤍

@SethFalco
Copy link
Member

Sorry, you had to wait so long for a review. I'm still triaging the backlog of issues/PRs in the project.

Last month, I updated the removeScriptElement plugin to remove all scripts. This will be renamed in v4 to removeScripts, and possibly an alias to be added sooner.

As a result, this has already been resolved. Once v3.0.3 is released, one can use removeScriptElement to drop <script> elements, event attributes, and JavaScript URIs.

Just a heads-up, this PR didn't cover all possible vectors for an XSS attack as it was missing JavaScript URIs, like the following:

<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 100 100">
  <a href="javascript:(() => { alert('uwu') })();">
    <text y="10">uwu</text>
  </a>
</svg>

See: #1807

@SethFalco SethFalco closed this Nov 4, 2023
@SethFalco
Copy link
Member

@pi0 You may want to review your implementation of removeXSS to make sure it handles JavaScript URIs as well until v3.0.3 is released.

Alternatively, you can wait up for the v3.0.3 release, but I can't guarantee a timeline. I've been prodding one of the maintainers to make a release soon as I don't have permissions, but everyone's busy. ^-^'

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

Successfully merging this pull request may close these issues.

None yet

4 participants