From 761cc688b4979e1f050572502eafd0c8ed9a3f4d Mon Sep 17 00:00:00 2001 From: Neil Lalonde Date: Fri, 21 Oct 2016 11:39:48 -0400 Subject: [PATCH] FEATURE: add a setting to allow url schemes other than http(s) --- .../javascripts/discourse/lib/text.js.es6 | 2 +- .../engines/discourse-markdown.js.es6 | 2 +- .../pretty-text/pretty-text.js.es6 | 3 ++- .../javascripts/pretty-text/sanitizer.js.es6 | 20 +++++++++++++++---- .../pretty-text/white-lister.js.es6 | 13 ++++++++---- config/locales/server.en.yml | 1 + config/site_settings.yml | 4 ++++ test/javascripts/mdtest/mdtest.js.es6.erb | 12 +++++++++-- 8 files changed, 44 insertions(+), 13 deletions(-) diff --git a/app/assets/javascripts/discourse/lib/text.js.es6 b/app/assets/javascripts/discourse/lib/text.js.es6 index 5a140b48c14..56f460852f2 100644 --- a/app/assets/javascripts/discourse/lib/text.js.es6 +++ b/app/assets/javascripts/discourse/lib/text.js.es6 @@ -19,7 +19,7 @@ export function cook(text) { } export function sanitize(text) { - return textSanitize(text, new WhiteLister(getOpts().features)); + return textSanitize(text, new WhiteLister(getOpts())); } function emojiOptions() { diff --git a/app/assets/javascripts/pretty-text/engines/discourse-markdown.js.es6 b/app/assets/javascripts/pretty-text/engines/discourse-markdown.js.es6 index a24375efef4..fbc58934b60 100644 --- a/app/assets/javascripts/pretty-text/engines/discourse-markdown.js.es6 +++ b/app/assets/javascripts/pretty-text/engines/discourse-markdown.js.es6 @@ -389,7 +389,7 @@ export function cook(raw, opts) { preProcessors.forEach(p => raw = p(raw)); - const whiteLister = new WhiteLister(opts.features); + const whiteLister = new WhiteLister(opts); const tree = parser.toHTMLTree(raw, 'Discourse'); let result = opts.sanitizer(parser.renderJsonML(parseTree(tree, opts)), whiteLister); diff --git a/app/assets/javascripts/pretty-text/pretty-text.js.es6 b/app/assets/javascripts/pretty-text/pretty-text.js.es6 index a70da34ccf5..2769502ff86 100644 --- a/app/assets/javascripts/pretty-text/pretty-text.js.es6 +++ b/app/assets/javascripts/pretty-text/pretty-text.js.es6 @@ -48,6 +48,7 @@ export function buildOptions(state) { getCurrentUser, currentUser, mentionLookup: state.mentionLookup, + allowedHrefSchemes: siteSettings.allowed_href_schemes ? siteSettings.allowed_href_schemes.split('|') : null }; _registerFns.forEach(fn => fn(siteSettings, options, state)); @@ -71,6 +72,6 @@ export default class { } sanitize(html) { - return this.opts.sanitizer(html, new WhiteLister(this.opts.features)); + return this.opts.sanitizer(html, new WhiteLister(this.opts)); } }; diff --git a/app/assets/javascripts/pretty-text/sanitizer.js.es6 b/app/assets/javascripts/pretty-text/sanitizer.js.es6 index a643169c399..d865c9be5ca 100644 --- a/app/assets/javascripts/pretty-text/sanitizer.js.es6 +++ b/app/assets/javascripts/pretty-text/sanitizer.js.es6 @@ -42,7 +42,7 @@ export function escape(string) { return string.replace(BAD_CHARS, escapeChar); } -export function hrefAllowed(href) { +export function hrefAllowed(href, extraHrefMatchers) { // escape single quotes href = href.replace(/'/g, "%27"); @@ -54,6 +54,12 @@ export function hrefAllowed(href) { if (/^#[\w\.\-]+/i.test(href)) { return href; } // mailtos if (/^mailto:[\w\.\-@]+/i.test(href)) { return href; } + + if (extraHrefMatchers && extraHrefMatchers.length > 0) { + for (let i=0; i 0) { + extraHrefMatchers = [new RegExp('^(' + allowedHrefSchemes.join('|') + '):\/\/[\\w\\.\\-]+','i')]; + } let result = xss(text, { whiteList: whiteList.tagList, @@ -75,8 +87,8 @@ export function sanitize(text, whiteLister) { const forAttr = forTag[name]; if ((forAttr && (forAttr.indexOf('*') !== -1 || forAttr.indexOf(value) !== -1)) || (name.indexOf('data-') === 0 && forTag['data-*']) || - ((tag === 'a' && name === 'href') && hrefAllowed(value)) || - (tag === 'img' && name === 'src' && (/^data:image.*$/i.test(value) || hrefAllowed(value))) || + ((tag === 'a' && name === 'href') && hrefAllowed(value, extraHrefMatchers)) || + (tag === 'img' && name === 'src' && (/^data:image.*$/i.test(value) || hrefAllowed(value, extraHrefMatchers))) || (tag === 'iframe' && name === 'src' && _validIframes.some(i => i.test(value)))) { return attr(name, value); } diff --git a/app/assets/javascripts/pretty-text/white-lister.js.es6 b/app/assets/javascripts/pretty-text/white-lister.js.es6 index 9c3a85439a0..bfe7b8a0b66 100644 --- a/app/assets/javascripts/pretty-text/white-lister.js.es6 +++ b/app/assets/javascripts/pretty-text/white-lister.js.es6 @@ -13,12 +13,13 @@ function concatUniq(src, elems) { } export default class WhiteLister { - constructor(features) { - features.default = true; + constructor(options) { + options.features.default = true; - this._featureKeys = Object.keys(features).filter(f => features[f]); + this._featureKeys = Object.keys(options.features).filter(f => options.features[f]); this._key = this._featureKeys.join(':'); - this._features = features; + this._features = options.features; + this._options = options||{}; } getCustom() { @@ -54,6 +55,10 @@ export default class WhiteLister { } return _whiteLists[this._key]; } + + getAllowedHrefSchemes() { + return this._options.allowedHrefSchemes || []; + } } // Builds our object that represents whether something is sanitized for a particular feature. diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index cfcab9a584e..2db6ecb65cc 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1332,6 +1332,7 @@ en: embed_username_key_from_feed: "Key to pull discourse username from feed." embed_title_scrubber: "Regular expression for scrubbing embeddable titles." embed_truncate: "Truncate the embedded posts." + allowed_href_schemes: "Schemes allowed in links in addition to http and https." embed_post_limit: "Maximum number of posts to embed." embed_username_required: "The username for topic creation is required." embed_whitelist_selector: "CSS selector for elements that are allowed in embeds." diff --git a/config/site_settings.yml b/config/site_settings.yml index 233600505ea..74f5116906d 100644 --- a/config/site_settings.yml +++ b/config/site_settings.yml @@ -567,6 +567,10 @@ posting: - code-fences embed_truncate: default: true + allowed_href_schemes: + client: true + default: '' + type: list email: email_time_window_mins: diff --git a/test/javascripts/mdtest/mdtest.js.es6.erb b/test/javascripts/mdtest/mdtest.js.es6.erb index a17d5645135..92047e74130 100644 --- a/test/javascripts/mdtest/mdtest.js.es6.erb +++ b/test/javascripts/mdtest/mdtest.js.es6.erb @@ -37,9 +37,9 @@ function sanitizer(result, whiteLister) { return result; } -function md(input, expected, text) { +function md(input, expected, text, settings) { - const opts = buildOptions({ siteSettings: {} }); + const opts = buildOptions({ siteSettings: settings||{} }); opts.traditionalMarkdownLinebreaks = true; opts.sanitizer = sanitizer; @@ -77,3 +77,11 @@ test("first", function(){ %> <%= mdtest_suite %> + +test("whitelisted url scheme", function() { + md("[Steam URL Scheme](steam://store/452530)", '

Steam URL Scheme

', 'whitelists the steam url', {allowed_href_schemes: "macappstore|steam"}); +}); + +test("forbidden url scheme", function() { + md("[Steam URL Scheme](steam://store/452530)", '

Steam URL Scheme

', 'removes the href', {allowed_href_schemes: "macappstore|itunes"}); +});