whitelist google.com/maps iframes

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.<?string>} The sanitized attributes as a list of alternating
    *     names and values, where a null value means to omit the attribute.
    */
-  function sanitizeAttribs(tagName, attribs,
+  function sanitizeAttribs(tagName, attribs, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) {
-    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.<?string>)} A tagPolicy suitable for
    *     passing to html.sanitize.
    */
-  function makeTagPolicy(
+  function makeTagPolicy(opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) {
-    opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) {
     return function(tagName, attribs) {
       if (!(html4.ELEMENTS[tagName] & html4.eflags['UNSAFE'])) {
-        return {
+        var sanitizedAttribs = sanitizeAttribs(tagName, attribs, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger);
-          'attribs': sanitizeAttribs(tagName, attribs,
+        var requiredAttributes = html4.REQUIREDATTRIBUTES[tagName];
-            opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger)
+        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,
+  function sanitize(inputHtml, opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) {
-    opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger) {
+    var tagPolicy = makeTagPolicy(opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger);
-    var tagPolicy = makeTagPolicy(
-      opt_naiveUriRewriter, opt_nmTokenPolicy, opt_logger);
     return sanitizeWithPolicy(inputHtml, tagPolicy);
   }
 

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() ];
     }
   }
-});
+});

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) {
+    args.forEach(function (a) { validClasses[a] = true; });
-      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 || raw.length === 0) return "";
-    if (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) {
+  urlAllowed: function (uri, effect, ltype, hints) {
-    if(/^https?:\/\//.test(url)) { return url; }
+    var url = typeof(uri) === "string" ? uri : uri.toString();
-    if(/^\/\/?[\w\.\-]+/.test(url)) { return url; }
+
+    // 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 ? "" : text;
-
-        return 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);

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("<table><tr><td>hello</td></tr></table>\nafter", "<p>after</p>", "it does not allow tables");
   cooked("<blockquote>a\n</blockquote>\n", "<blockquote>a\n\n<br/>\n\n</blockquote>", "it does not double sanitize");
+
+  cooked("<iframe src=\"http://discourse.org\" width=\"100\" height=\"42\"></iframe>", "", "it does not allow most iframe");
+
+  cooked("<iframe src=\"https://www.google.com/maps/embed?pb=!1m10!1m8!1m3!1d2624.9983685732213!2d2.29432085!3d48.85824149999999!3m2!1i1024!2i768!4f13.1!5e0!3m2!1sen!2s!4v1385737436368\" width=\"100\" height=\"42\"></iframe>",
+         "<iframe src=\"https://www.google.com/maps/embed?pb=!1m10!1m8!1m3!1d2624.9983685732213!2d2.29432085!3d48.85824149999999!3m2!1i1024!2i768!4f13.1!5e0!3m2!1sen!2s!4v1385737436368\" width=\"100\" height=\"42\"></iframe>",
+         "it allows iframe to google maps");
 });
 
 test("URLs in BBCode tags", function() {