From 9b6538832d464655dfe5c530137cebe88c9a60d7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?R=C3=A9gis=20Hanol?= Date: Fri, 29 Nov 2013 18:08:53 +0100 Subject: [PATCH] whitelist google.com/maps iframes --- .../defer/html-sanitizer-bundle.js | 46 +++++++++++++------ .../javascripts/discourse/dialects/html.js | 4 +- .../javascripts/discourse/lib/markdown.js | 43 +++++++++++------ test/javascripts/lib/markdown_test.js | 12 ++--- 4 files changed, 70 insertions(+), 35 deletions(-) diff --git a/app/assets/javascripts/defer/html-sanitizer-bundle.js b/app/assets/javascripts/defer/html-sanitizer-bundle.js index 146fad1cf..ff1973c62 100644 --- a/app/assets/javascripts/defer/html-sanitizer-bundle.js +++ b/app/assets/javascripts/defer/html-sanitizer-bundle.js @@ -409,7 +409,7 @@ URI.prototype.setPath = function (newPath) { URI.prototype.setRawPath = function (newPath) { if (newPath) { newPath = String(newPath); - this.path_ = + this.path_ = // Paths must start with '/' unless this is a path-relative URL. (!this.domain_ || /^\//.test(newPath)) ? newPath : '/' + newPath; } else { @@ -898,6 +898,7 @@ html4.ATTRIBS = { 'iframe::marginheight': 0, 'iframe::marginwidth': 0, 'iframe::width': 0, + 'iframe::src': 1, 'img::align': 0, 'img::alt': 0, 'img::border': 0, @@ -1293,6 +1294,7 @@ html4.URIEFFECTS = { 'command::icon': 1, 'del::cite': 0, 'form::action': 2, + 'iframe::src': 1, 'img::src': 1, 'input::src': 1, 'ins::cite': 0, @@ -1315,6 +1317,7 @@ html4.LOADERTYPES = { 'command::icon': 1, 'del::cite': 2, 'form::action': 2, + 'iframe::src': 2, 'img::src': 1, 'input::src': 1, 'ins::cite': 2, @@ -1323,6 +1326,15 @@ html4.LOADERTYPES = { 'video::src': 2 }; html4[ 'LOADERTYPES' ] = html4.LOADERTYPES; +// NOTE: currently focused only on URI-type attributes +html4.REQUIREDATTRIBUTES = { + "audio" : ["src"], + "form" : ["action"], + "iframe" : ["src"], + "image" : ["src"], + "video" : ["src"] +}; +html4[ 'REQUIREDATTRIBUTES' ] = html4.REQUIREDATTRIBUTES; // export for Closure Compiler if (typeof window !== 'undefined') { window['html4'] = html4; @@ -2194,8 +2206,7 @@ var html = (function(html4) { * @return {Array.} The sanitized attributes as a list of alternating * names and values, where a null value means to omit the attribute. */ - function sanitizeAttribs(tagName, attribs, - opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { + function sanitizeAttribs(tagName, attribs, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { // TODO(felix8a): it's obnoxious that domado duplicates much of this // TODO(felix8a): maybe consistently enforce constraints like target= for (var i = 0; i < attribs.length; i += 2) { @@ -2277,7 +2288,7 @@ var html = (function(html4) { "XML_ATTR": attribName, "XML_TAG": tagName }, opt_naiveUriRewriter); - if (opt_logger) { + if (opt_logger) { log(opt_logger, tagName, attribName, oldValue, value); } break; @@ -2325,14 +2336,13 @@ var html = (function(html4) { * @return {function(string, Array.)} A tagPolicy suitable for * passing to html.sanitize. */ - function makeTagPolicy( - opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { + function makeTagPolicy(opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { return function(tagName, attribs) { if (!(html4.ELEMENTS[tagName] & html4.eflags['UNSAFE'])) { - return { - 'attribs': sanitizeAttribs(tagName, attribs, - opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) - }; + var sanitizedAttribs = sanitizeAttribs(tagName, attribs, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger); + var requiredAttributes = html4.REQUIREDATTRIBUTES[tagName]; + if (requiredAttributes && missRequiredAttributes(sanitizedAttribs, requiredAttributes)) { return } + return { 'attribs': sanitizedAttribs }; } else { if (opt_logger) { log(opt_logger, tagName, undefined, undefined, undefined); @@ -2341,6 +2351,16 @@ var html = (function(html4) { }; } + function missRequiredAttributes(sanitizedAttributes, requiredAttributes) { + var requiredAttributesWithValueCount = 0; + for (var i = 0, length = sanitizedAttributes.length; i < length; i += 2) { + var name = sanitizedAttributes[i]; + var value = sanitizedAttributes[i + 1]; + if (requiredAttributes.indexOf(name) > -1 && value && value.length > 0) { requiredAttributesWithValueCount++; } + } + return requiredAttributesWithValueCount != requiredAttributes.length; + } + /** * Sanitizes HTML tags and attributes according to a given policy. * @param {string} inputHtml The HTML to sanitize. @@ -2364,10 +2384,8 @@ var html = (function(html4) { * to attributes containing HTML names, element IDs, and space-separated * lists of classes. If not given, such attributes are left unchanged. */ - function sanitize(inputHtml, - opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { - var tagPolicy = makeTagPolicy( - opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger); + function sanitize(inputHtml, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) { + var tagPolicy = makeTagPolicy(opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger); return sanitizeWithPolicy(inputHtml, tagPolicy); } diff --git a/app/assets/javascripts/discourse/dialects/html.js b/app/assets/javascripts/discourse/dialects/html.js index d0682b5e1..ed4d4641a 100644 --- a/app/assets/javascripts/discourse/dialects/html.js +++ b/app/assets/javascripts/discourse/dialects/html.js @@ -3,7 +3,7 @@ **/ var blockTags = ['address', 'article', 'aside', 'audio', 'blockquote', 'canvas', 'dd', 'div', 'dl', 'fieldset', 'figcaption', 'figure', 'footer', 'form', 'h1', 'h2', 'h3', - 'h4', 'h5', 'h6', 'header', 'hgroup', 'hr', 'noscript', 'ol', 'output', + 'h4', 'h5', 'h6', 'header', 'hgroup', 'hr', 'iframe', 'noscript', 'ol', 'output', 'p', 'pre', 'section', 'table', 'tfoot', 'ul', 'video'], splitAtLast = function(tag, block, next, first) { @@ -39,4 +39,4 @@ Discourse.Dialect.registerBlock('html', function(block, next) { return [ block.toString() ]; } } -}); \ No newline at end of file +}); diff --git a/app/assets/javascripts/discourse/lib/markdown.js b/app/assets/javascripts/discourse/lib/markdown.js index d702d7df2..b7c37b856 100644 --- a/app/assets/javascripts/discourse/lib/markdown.js +++ b/app/assets/javascripts/discourse/lib/markdown.js @@ -10,6 +10,7 @@ Discourse.Markdown = { validClasses: {}, + validIframes: [], /** Whitelists classes for sanitization @@ -21,9 +22,17 @@ Discourse.Markdown = { var args = Array.prototype.slice.call(arguments), validClasses = Discourse.Markdown.validClasses; - args.forEach(function (a) { - validClasses[a] = true; - }); + args.forEach(function (a) { validClasses[a] = true; }); + }, + + /** + Whitelists iframes for sanitization + + @method whiteListIframe + @param {Regexp} regexp The regexp to whitelist. + **/ + whiteListIframe: function(regexp) { + Discourse.Markdown.validIframes.push(regexp); }, /** @@ -38,8 +47,7 @@ Discourse.Markdown = { if (!opts) opts = {}; // Make sure we've got a string - if (!raw) return ""; - if (raw.length === 0) return ""; + if (!raw || raw.length === 0) return ""; return this.markdownConverter(opts).makeHtml(raw); }, @@ -52,7 +60,6 @@ Discourse.Markdown = { @return {Markdown.Editor} the editor instance **/ createEditor: function(converterOptions) { - if (!converterOptions) converterOptions = {}; // By default we always sanitize content in the editor @@ -109,9 +116,21 @@ Discourse.Markdown = { @param {String} url Url to check @return {String} url to insert in the cooked content **/ - urlAllowed: function (url) { - if(/^https?:\/\//.test(url)) { return url; } - if(/^\/\/?[\w\.\-]+/.test(url)) { return url; } + urlAllowed: function (uri, effect, ltype, hints) { + var url = typeof(uri) === "string" ? uri : uri.toString(); + + // whitelist some iframe only + if (hints && hints.XML_TAG === "iframe" && hints.XML_ATTR === "src") { + for (var i = 0, length = Discourse.Markdown.validIframes.length; i < length; i++) { + if(Discourse.Markdown.validIframes[i].test(url)) { return url; } + } + return; + } + + // absolute urls + if(/^(https?:)?\/\/[\w\.\-]+/i.test(url)) { return url; } + // relative urls + if(/^\/[\w\.\-]+/i.test(url)) { return url; } }, /** @@ -149,11 +168,8 @@ Discourse.Markdown = { return { makeHtml: function(text) { - text = Discourse.Dialect.cook(text, opts); - if (!text) return ""; - - return text; + return !text ? "" : text; } }; } @@ -162,3 +178,4 @@ Discourse.Markdown = { RSVP.EventTarget.mixin(Discourse.Markdown); Discourse.Markdown.whiteListClass("attachment"); +Discourse.Markdown.whiteListIframe(/^(https?:)?\/\/www\.google\.com\/maps\/embed\?.+/i); diff --git a/test/javascripts/lib/markdown_test.js b/test/javascripts/lib/markdown_test.js index d3c3ba1b7..a3e3d0499 100644 --- a/test/javascripts/lib/markdown_test.js +++ b/test/javascripts/lib/markdown_test.js @@ -8,12 +8,6 @@ module("Discourse.Markdown", { var cooked = function(input, expected, text) { var result = Discourse.Markdown.cook(input, {mentionLookup: false, sanitize: true}); - - if (result !== expected) { - console.log(JSON.stringify(result)); - console.log(JSON.stringify(expected)); - } - equal(result, expected, text); }; @@ -337,6 +331,12 @@ test("sanitize", function() { cooked("
hello
\nafter", "

after

", "it does not allow tables"); cooked("
a\n
\n", "
a\n\n
\n\n
", "it does not double sanitize"); + + cooked("", "", "it does not allow most iframe"); + + cooked("", + "", + "it allows iframe to google maps"); }); test("URLs in BBCode tags", function() {