diff --git a/src/sanitizer.js b/src/sanitizer.js index 46fdc68f05c8..5abaa131acc0 100644 --- a/src/sanitizer.js +++ b/src/sanitizer.js @@ -33,25 +33,32 @@ const SELF_CLOSING_TAGS = { }; -/** @const {!Object} */ -const WHITELISTED_FORMAT_TAGS = { - 'b': true, - 'br': true, - 'code': true, - 'del': true, - 'em': true, - 'i': true, - 'ins': true, - 'mark': true, - 'q': true, - 's': true, - 'small': true, - 'strong': true, - 'sub': true, - 'sup': true, - 'time': true, - 'u': true, -}; +/** @const {!Array} */ +const WHITELISTED_FORMAT_TAGS = [ + 'b', + 'br', + 'code', + 'del', + 'em', + 'i', + 'ins', + 'mark', + 'q', + 's', + 'small', + 'strong', + 'sub', + 'sup', + 'time', + 'u', +]; + + +/** @const {!Array} */ +const BLACKLISTED_ATTR_VALUES = [ + /*eslint no-script-url: 0*/ 'javascript:', + /*eslint no-script-url: 0*/ 'vbscript:', +]; /** @@ -132,7 +139,7 @@ export function sanitizeHtml(html) { */ export function sanitizeFormattingHtml(html) { return htmlSanitizer.sanitizeWithPolicy(html, function(tagName, attrs) { - if (!WHITELISTED_FORMAT_TAGS[tagName]) { + if (WHITELISTED_FORMAT_TAGS.indexOf(tagName) == -1) { return null; } return { @@ -161,10 +168,12 @@ export function isValidAttr(attrName, attrValue) { return false; } - // No attributes with "javascript" in them. - if (attrValue.toLowerCase().indexOf( - /*eslint no-script-url: 0*/ 'javascript:') != -1) { - return false; + // No attributes with "javascript" or other blacklisted substrings in them. + const attrValueLowercase = attrValue.toLowerCase(); + for (let i = 0; i < BLACKLISTED_ATTR_VALUES.length; i++) { + if (attrValueLowercase.indexOf(BLACKLISTED_ATTR_VALUES[i]) != -1) { + return false; + } } return true; diff --git a/test/functional/test-mustache.js b/test/functional/test-mustache.js index e53cbbc8ccbf..f01533a48687 100644 --- a/test/functional/test-mustache.js +++ b/test/functional/test-mustache.js @@ -40,4 +40,32 @@ describe('Mustache', () => { expect(mustache.render('{{{value}}}', {value: 'abc'})).to.equal( 'ABC'); }); + + it('should only expand own properties', () => { + const parent = {value: 'bc'}; + const child = Object.create(parent); + const container = {parent: parent, child: child}; + expect(mustache.render('a{{value}}', parent)).to.equal('abc'); + expect(mustache.render('a{{value}}', child)).to.equal('a'); + expect(mustache.render('a{{parent.value}}', container)).to.equal('abc'); + expect(mustache.render('a{{child.value}}', container)).to.equal('a'); + }); + + it('should NOT allow calls to builtin functions', () => { + // Calls on x.pop in classical Mustache would lead to builtin call and + // mutate on the 't' object. Here we will not allow it. We explicitly + // prohibit such calls. + const obj = { + 't': { + '0': '0', + '1': '1', + 'length': 2, + 'x': [] + } + }; + expect(mustache.render( + '{{#t}}{{x.pop}}X{{x.pop}}{{/t}}' + + '{{#t}}{{0}}Y{{1}}{{/t}}', + obj)).to.equal('X0Y1'); + }); }); diff --git a/test/functional/test-sanitizer.js b/test/functional/test-sanitizer.js index c25a419b691c..258fc8012f1e 100644 --- a/test/functional/test-sanitizer.js +++ b/test/functional/test-sanitizer.js @@ -79,6 +79,10 @@ describe('sanitizeHtml', () => { 'ab'); expect(sanitizeHtml('ab')).to.be.equal( 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); + expect(sanitizeHtml('ab')).to.be.equal( + 'ab'); }); it('should NOT output security-sensitive attributes', () => { diff --git a/third_party/mustache/mustache.js b/third_party/mustache/mustache.js index 9cd3bb2b04dc..0226e2dbae6c 100644 --- a/third_party/mustache/mustache.js +++ b/third_party/mustache/mustache.js @@ -42,7 +42,8 @@ * including its prototype, has a given property */ function hasProperty (obj, propName) { - return obj != null && typeof obj === 'object' && (propName in obj); + return obj != null && typeof obj === 'object' && + Object.prototype.hasOwnProperty.call(obj, propName); } // Workaround for https://issues.apache.org/jira/browse/COUCHDB-577 @@ -397,14 +398,21 @@ * `undefined` and we want to avoid looking up parent contexts. **/ while (value != null && index < names.length) { + if (!hasProperty(value, names[index])) { + value = null; + break; + } if (index === names.length - 1) - lookupHit = hasProperty(value, names[index]); - + lookupHit = true; value = value[names[index++]]; } } else { - value = context.view[name]; - lookupHit = hasProperty(context.view, name); + if (!hasProperty(context.view, name)) { + value = null; + } else { + value = context.view[name]; + lookupHit = true; + } } if (lookupHit)