Skip to content
This repository has been archived by the owner on Jul 15, 2019. It is now read-only.

null replacement option available #48

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

adon-at-work
Copy link
Contributor

This makes available an advanced config feature to make filter replace NULL. By using xssFilters._privFilters.config({replaceNull:true}), the five most basic contextual filters, on which all contexts/filters depend, will be replacing the null character with \uFFFD at last.

The performance penalty of enabling this is detailed in https://github.com/yahoo/xss-filters/blob/benchmarks/tests/benchmarks/null-replacement.js

An alternative approach is to have a layer of polyfill that patches problematic IE's document.write() and document.writeln(). note that innerHTML has no such problem.

Coverage is also made 100% thru this PR.

Coverage is also made 100%

Statements   : 100% ( 113/113 )
Branches     : 100% ( 175/175 )
Functions    : 100% ( 51/51 )
Lines        : 100% ( 112/112 )
@adon-at-work adon-at-work self-assigned this Aug 6, 2015
@adon-at-work
Copy link
Contributor Author

@neraliu , please help review.

Kindly compare 5881d78 with 15d9d43. We can do either one or both.

@@ -24,7 +24,7 @@ exports._getPrivFilters = function () {
// By CSS: (Tab|NewLine|colon|semi|lpar|rpar|apos|sol|comma|excl|ast|midast);|(quot|QUOT)
// By URI_PROTOCOL: (Tab|NewLine);
var SENSITIVE_HTML_ENTITIES = /&(?:#([xX][0-9A-Fa-f]+|\d+);?|(Tab|NewLine|colon|semi|lpar|rpar|apos|sol|comma|excl|ast|midast|ensp|emsp|thinsp);|(nbsp|amp|AMP|lt|LT|gt|GT|quot|QUOT);?)/g,
SENSITIVE_NAMED_REF_MAP = {Tab: '\t', NewLine: '\n', colon: ':', semi: ';', lpar: '(', rpar: ')', apos: '\'', sol: '/', comma: ',', excl: '!', ast: '*', midast: '*', ensp: '\u2002', emsp: '\u2003', thinsp: '\u2009', nbsp: '\xA0', amp: '&', lt: '<', gt: '>', quot: '"', QUOT: '"'};
SENSITIVE_NAMED_REF_MAP = {Tab: '\t', NewLine: '\n', colon: ':', semi: ';', lpar: '(', rpar: ')', apos: '\'', sol: '/', comma: ',', excl: '!', ast: '*', midast: '*', ensp: '\u2002', emsp: '\u2003', thinsp: '\u2009', nbsp: '\xA0', amp: '&', AMP: '&', lt: '<', LT: '<', gt: '>', GT: '>', quot: '"', QUOT: '"'};
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why only lt etc. is case sensitive? how about other pattern like Lt etc?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

According to http://dev.w3.org/html5/html-author/charref, Lt is not a valid charref. Did you find it accepted by any browsers?
those decoding of & < > " are actually security non-critical, as no regexp is trying to match them. they're there only for those who're interested in using the html decoder (i.e., _privFilters.d()).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

as per an offline discussion. adding more comments to explain this list.

@adon-at-work adon-at-work force-pushed the make-null-replacement-configurable branch from 0fa3f02 to eeac3db Compare August 10, 2015 07:57
@adon-at-work adon-at-work force-pushed the make-null-replacement-configurable branch from eeac3db to 8acc660 Compare August 10, 2015 08:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants