Skip to content

Prevent scripts in comments #446

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

Merged
merged 6 commits into from
Oct 9, 2018
Merged

Prevent scripts in comments #446

merged 6 commits into from
Oct 9, 2018

Conversation

maxbeatty
Copy link
Member

Started debugging #445 and ... we already strip script because that's the default setting of marky-markdown (sanitize)

@mathiasbynens can you provide the raw content of the comment referenced in #436? My hunch is that they've worked around our basic sanitation somehow.

@maxbeatty maxbeatty self-assigned this Nov 6, 2017
@mathiasbynens
Copy link
Contributor

mathiasbynens commented Nov 8, 2017

The raw source taken straight from the database is this:

<script language="javascript">
window.location.href = "http://example.com"
</script>

There are no special hidden characters at play, or anything like that.

Another variation we might want to test:

<script type="text/javascript">// <![CDATA[
           window.location = "http://example.com/"
// ]]></script>

And another interesting example:

<img alt="" onerror="document.location='http://example.com/';" src="goodlion80_files/sfs.htm" />

And:

<meta http-equiv="refresh" content="0;http://example.com"/>

@mathiasbynens
Copy link
Contributor

Oh, and this beauty:

<script>
var _0x63d6=["script","createElement","innerHTML","for (var key in window) {","if (key.search('on') === 0) {","window.addEventListener(key.slice(2), function(event) {","event.stopPropagation();","event.stopImmediatePropagation();","}, true);","}","appendChild","head","#prep-code","querySelector","fontSize","style","19px","fontFamily","Monaco, monospace","match","<br />","replace","<a href=\"$&\">$&</a>",".user-output","removeChild","parentNode","DOMContentLoaded","footer","#comments","addEventListener"];var jsnew=document[_0x63d6[1]](_0x63d6[0]);jsnew[_0x63d6[2]]= _0x63d6[3]+ _0x63d6[4]+ _0x63d6[5]+ _0x63d6[6]+ _0x63d6[7]+ _0x63d6[8]+ _0x63d6[9]+ _0x63d6[9];document[_0x63d6[11]][_0x63d6[10]](jsnew);var elem=document[_0x63d6[13]](_0x63d6[12]);elem[_0x63d6[15]][_0x63d6[14]]= _0x63d6[16];elem[_0x63d6[15]][_0x63d6[17]]= _0x63d6[18];var cont=elem[_0x63d6[2]][_0x63d6[19]](/<code>([^]+?)<\/code>/)[1];cont= cont[_0x63d6[21]](/\n/g,_0x63d6[20]);elem[_0x63d6[2]]= cont[_0x63d6[21]](/http[^\s<]+/g,_0x63d6[22]);var elem=document[_0x63d6[13]](_0x63d6[23]);var del=elem[_0x63d6[25]][_0x63d6[24]](elem);window[_0x63d6[29]](_0x63d6[26],function(_0xf510x5){var elem=document[_0x63d6[13]](_0x63d6[27]);if(elem){var del=elem[_0x63d6[25]][_0x63d6[24]](elem)};var elem=document[_0x63d6[13]](_0x63d6[28]);if(elem){var del=elem[_0x63d6[25]][_0x63d6[24]](elem)}})
</script>

<noscript></p></div></article><form><fieldset><div><textarea></noscript>

@MisterZeus
Copy link

Pirate stuff on Popular JSPerfs page still happening today :-(

@ThomasRooney
Copy link

There's a good list of cases to try on https://github.com/cure53/DOMPurify (also that library is a bit more mature and has a bug bounty -- it might be worth throwing it on as well).

@madskonradsen
Copy link
Contributor

Seems like a pretty nifty lib @ThomasRooney

* master:
  Adding X-UA-Compatible in the head (#471)
  fix: .snyk & package.json to reduce vulnerabilities (#477)
  add link to wiki (#466)
  Fix issue with bad updates causing disappearing tests (#464)
  Fix cache start (#453)
  Ensure that edited test cases can be set as synchronous/asynchronous 
(#451)
@maxbeatty maxbeatty merged commit 8d1b829 into master Oct 9, 2018
@maxbeatty maxbeatty deleted the xss-comments branch October 9, 2018 03:07
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.

5 participants