🌐 AI搜索 & 代理 主页
Skip to content

Conversation

@fhemberger
Copy link
Contributor

Your version of the script is prone to DOM XSS attacks:
parseHTML('<img src=x onerror=alert(1)>') will execute the script immediately. This altered version prevents the execution of the script.

$.parseHTML('<img src=x onerror=alert(1)>') is still vulnerable to this attack, so generally it's not recommended to parse arbitrary HTML on the client side.

Your version of the script is prone to DOM XSS attacks:
`parseHTML('<img src=x onerror=alert(1)>')` will execute the script immediately. This altered version prevents the execution of the script.

`$.parseHTML('<img src=x onerror=alert(1)>')` is still vulnerable to this attack, so generally it's not recommended to parse arbitrary HTML on the client side.
@zackbloom
Copy link
Contributor

This is true, but, as you said, our solution doesn't differ from jQuery's. There is a safe way to parse HTML, but IIRC, it's not supported by Chrome.

@zackbloom zackbloom closed this Jan 31, 2014
@fhemberger
Copy link
Contributor Author

Look at the PR attached, there is the safer version. And it works perfectly on Chrome.

Please don't let people use the vulnerable version, it's already on StackOverflow all over the place. It is really harmful. I'll also open a issue on jQuery itself, maybe this issue can be fixed there as well.

@zackbloom zackbloom reopened this Jan 31, 2014
@zackbloom
Copy link
Contributor

Sorry, I completely missed that it was a PR.

This won't work in ie8, so please create an ie8.js with the original solution, and move yours to ie9.js.

@fhemberger
Copy link
Contributor Author

It might be possible to get this running on IE8 as well, I'll look into it and update the PR later.

@fhemberger
Copy link
Contributor Author

Ok, I added the improved IE8 compatible version as well.

@zackbloom
Copy link
Contributor

Wow, that solution is unexpected, I'm tempted to just leave the insecure version for ie8, as I don't think I can in good conscious tell people they should be creating an ActivX control to avoid using jQuery. In any case, ie8 solution needs to support ie8+, so add a fallback to the ie9+ method, and we're good to go.

@fhemberger
Copy link
Contributor Author

… but you can in good conscious tell them to use code which is easily vulnerable to XSS? I'd definitely prefer the ActiveX version then. This is AFAIK the only safe way in IE<=8 to do this.

@zackbloom
Copy link
Contributor

If trusted input is being parsed, there's nothing wrong with it. Without knowing how it's being used, I can't say that you must, or even should, use an ActiveX control to replicate jQuery's behavior.

@fhemberger
Copy link
Contributor Author

Ok, I'll update the PR offering both solutions for IE8 with a warning that one is vulnerable to XSS and should only be used with caution. Are you fine with this?

@fhemberger
Copy link
Contributor Author

Ok, now I listed both versions.

@zackbloom zackbloom merged commit d41c6ad into HubSpot:gh-pages Jan 31, 2014
@zackbloom
Copy link
Contributor

I merged it without the ActiveX version. If the jQuery version was secure in this way, it would be a different story, but the ultimate point of this website is to showcase how to replicate jQuery's functionality, not the many improvements which could be made to it.

Thank you for your contribution, I hope improving security for sites targeting IE9 and above is good enough.

@fhemberger
Copy link
Contributor Author

Then please re-add at least the XSS warning to both jQuery and IE8. I'll file this issue for jQuery as well.

@fhemberger
Copy link
Contributor Author

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.

2 participants