XSS CS Update

[rdela]

This PR covers issue #101

Update JS/Node.js libs in Cross Site Scripting Prevention Rule 6 "Other libraries that provide HTML Sanitization":

Line 344 linked to ecto/bleach as "JavaScript/Node.js Bleach"

Now Line 344-5 link to:

• cure53/DOMPurify, a DOM-only library that requires JSDOM on Node.js
• punkave/sanitize-html, a Node.js-only library that depends on htmlparser2

...as:

• JavaScript (DOM-only, requires JSDOM for Node.js)
• Node.js (depends on htmlparser2)

[righettod]

OK for me.
Thank you very much for the PR 😃

[mackowski]

Thanks for this PR.

I like that you added DOMPurify from cure53 (in fact I am very surprised that it was not there)
I am even more happy that you spotted old, unsupported library on that list.
But I think that we should not add here https://github.com/punkave/sanitize-html.

Libraries on that list should be designed to clean potentially malicious HTML from untrusted source to prevent XSS. HTML sanitizer from Closure, PHP HTML Purifier, Python Bleach and DOMPurify from cure53 do that but I never heard of https://github.com/punkave/sanitize-html.

Documentation for this library tells that "sanitize-html is tolerant. It is well suited for cleaning up HTML fragments such as those created by ckeditor and other rich text editors. It is especially handy for removing unwanted CSS when copying and pasting from Word.". This is not our use case. IIn addition, from what I saw, it does not sanitize javascript code by default. User have to specify what should be allowed (tags, attubutes, etc.) and what not. Summarising this is not security library and we cannot guarantee that it will protect users from XSS.

As I wrote before I never heard of this library and I only checked documentation so if I am wrong somewhere and this library can delete javascript please tell me :)

[rdela]

First off, I completely defer to your decisions about what goes in. I wrote in #101:

My cursory research into currently maintained libraries yielded two options [...] If OWASP contributors find both libraries adequate and satisfactory [...]

If the security posture of punkave/sanitize-html is not sufficiently intolerant and obvious enough to warrant inclusion, then I think we should leave sanitize-html out and encourage Node.js devs to use DOMPurify with JSDOM. The fact that you have heard of DOMPurify and were surprised it was not there speaks volumes.

That said, sanitize-html does remove JS. From the README opening section:

If a tag is not permitted, the contents of the tag are still kept, except for script, style and textarea tags.

Further down in "What are the default options?":

allowedTags: [ 'h3', 'h4', 'h5', 'h6', 'blockquote', 'p', 'a', 'ul', 'ol',
  'nl', 'li', 'b', 'i', 'strong', 'em', 'strike', 'code', 'hr', 'br', 'div',
  'table', 'thead', 'caption', 'tbody', 'tr', 'th', 'td', 'pre', 'iframe' ],
allowedAttributes: {
  a: [ 'href', 'name', 'target' ],
  // We don't currently allow img itself by default, but this
  // would make sense if we did. You could add srcset here,
  // and if you do the URL is checked for safety
  img: [ 'src' ]
},
// Lots of these won't come up by default because we don't allow them
selfClosing: [ 'img', 'br', 'hr', 'area', 'base', 'basefont', 'input', 'link', 'meta' ],
// URL schemes we permit
allowedSchemes: [ 'http', 'https', 'ftp', 'mailto' ],
allowedSchemesByTag: {},
allowedSchemesAppliedToAttributes: [ 'href', 'src', 'cite' ],
allowProtocolRelative: true

Later on:

Allowed URL schemes

By default we allow the following URL schemes in cases where href, src, etc. are allowed:

[ 'http', 'https', 'ftp', 'mailto' ]

You can override this if you want to:

sanitizeHtml(
  // teeny-tiny valid transparent GIF in a data URL
  '<img src="data:image/gif;base64,R0lGODlhAQABAAAAACH5BAEKAAEALAAAAAABAAEAAAICTAEAOw==" />',
  {
    allowedTags: [ 'img', 'p' ],
    allowedSchemes: [ 'data', 'http' ]
  }
);

You can also allow a scheme for a particular tag only:

allowedSchemes: [ 'http', 'https' ],
allowedSchemesByTag: {
  img: [ 'data' ]
}

And you can forbid the use of protocol-relative URLs (starting with //) to access another site using the current protocol, which is allowed by default:

allowProtocolRelative: false

Discarding the entire contents of a disallowed tag

Normally, with a few exceptions, if a tag is not allowed, all of the text within it is preserved, and so are any allowed tags within it.

The exceptions are:

style, script, textarea

If you wish to expand this list, for instance to discard whatever is found inside a noscript tag, use the nonTextTags option:

nonTextTags: [ 'style', 'script', 'textarea', 'noscript' ]

Note that if you use this option you are responsible for stating the entire list. This gives you the power to retain the content of textarea, if you want to.

The content still gets escaped properly, with the exception of the script and style tags. Allowing either script or style leaves you open to XSS attacks. Don't do that unless you have good reason to trust their origin.

So by default...

• script, style, and textarea tags are removed and the entire contents are discarded.
• All attributes are removed from all tags except href, name, and target on a tags. On href only http, https, ftp, and mailto protocols are allowed.
• iframe tags are allowed by default but the src attribute is not. There is an open issue about this from 10 April where contributor Tom Boutell suggests iframe will be removed in the 2.x release since it is a breaking change and says:

  as it currently stands the configuration does not make much sense because iframe is allowed, but src is not allowed for iframe.

Thought I would clarify all that to clear up any misunderstanding, but I will add a commit that removes sanitize-html on Monday unless I hear differently from @mackowski before then.

[mackowski]

Thank you for detailed answer!
I was obviously wrong and this library does sanitize javascript/xss paylods by default.
Many thanks for clarification.
DOMPurify and Google Closure are well know and de facto standard javascript libs for this job.
But the fact that I didn't know about sanitize-html does not mean that this is a wrong library!
On one side I am not sure if we have to have 3 js libs but on the other hand why not :)

@ThunderSon what do you think?

[rdela]

I am all for sticking to the de facto standard libs already familiar to OWASP contributors. While we are on the subject, should we rearrange the order to keep languages together and possibly add (JavaScript/Node.js) to the end of the Closure listing to be super explicit?

Like so:

Other libraries that provide HTML Sanitization include:

• HTML sanitizer from Google Closure Library (JavaScript/Node.js)
• DOMPurify (JavaScript, DOM-only, requires JSDOM for Node.js)
• PHP HTML Purifier
• Python Bleach

[rdela]

Looks like maybe we should not capitalize jsdom either, and maybe we should lose "DOM-only" and link that too?

So:

Other libraries that provide HTML Sanitization include:

• HTML sanitizer from Google Closure Library (JavaScript/Node.js)
• DOMPurify (JavaScript, requires jsdom for Node.js)
• PHP HTML Purifier
• Python Bleach

[rdela]

We could also probably save lazy devs tons of time by linking to the HTML sanitizer docs, not easy to find from the main Closure link. Although https://github.com/google/closure-library/blob/master/closure/goog/html/sanitizer/htmlsanitizer.js has a simple usage example currently on Line 23.

So maybe adjust HTML sanitizer link to htmlsanitizer.js and add docs link at end?

Other libraries that provide HTML Sanitization include:

• HTML sanitizer from Google Closure Library (JavaScript/Node.js, docs)
• DOMPurify (JavaScript, requires jsdom for Node.js)
• PHP HTML Purifier
• Python Bleach

[jmanico]

Please put https://www.owasp.org/index.php/OWASP_Java_HTML_Sanitizer_Project back it. It’s actively maintained by Mike Samuel via Google. Aloha, -- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

…

On May 10, 2019, at 3:26 PM, Ricky de Laveaga ***@***.***> wrote: I am all for sticking to the de facto standard libs already familiar to OWASP contributors. While we are on the subject, should we rearrange the order to keep languages together and possibly add (JavaScript/Node.js) to the end of the Closure listing to be super explicit? Like so: Other libraries that provide HTML Sanitization include: HTML sanitizer from Google Closure Library (JavaScript/Node.js) DOMPurify (JavaScript, DOM-only, requires JSDOM for Node.js) PHP HTML Purifier Python Bleach — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[jmanico]

Remember these are cheats. Please err on the side of brevity. Make sure to link vs include extra content.

…

-- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

[mackowski]

No worries Jim we did not deleted https://www.owasp.org/index.php/OWASP_Java_HTML_Sanitizer_Project it is recommended for Java :)

[mackowski]

@rdela I like your ideas about rearrange the order and improve linking to the closure docs and example.

[jmanico]

Woot!!!!

…

-- Jim Manico @manicode Secure Coding Education +1 (808) 652-3805

[rdela]

@mackowski Updated with suggested changes from my last comment #103 (comment)

Travis build is failing but looks like that is only missing newlines in other cheatsheets?
https://travis-ci.com/OWASP/CheatSheetSeries/builds/111902594#L510

Want me to add them?

[righettod]

Give some time, i will fix it.
Ticket #105 assigned to me

[righettod]

I have added info about the TravisCI job issue to fix it in existing PR.

[rdela]

Thanks @righettod! PR updated and Travis is passing now ✅

[mackowski]

Awesome thanks!