Merge pull request #1359 from aprotim/fix_url_sanitization

Make URL handling consistent between links and images
markedjs · Oct 19, 2018 · 3bc4b99 · 3bc4b99
2 parents da9d155 + 2353d95
commit 3bc4b99
Show file tree

Hide file tree

Showing 3 changed files with 47 additions and 20 deletions.
diff --git a/lib/marked.js b/lib/marked.js
@@ -1034,24 +1034,8 @@ Renderer.prototype.del = function(text) {
 };
 
 Renderer.prototype.link = function(href, title, text) {
- if (this.options.sanitize) {
- try {
- var prot = decodeURIComponent(unescape(href))
- .replace(/[^\w:]/g, '')
- .toLowerCase();
- } catch (e) {
- return text;
- }
- if (prot.indexOf('javascript:') === 0 || prot.indexOf('vbscript:') === 0 || prot.indexOf('data:') === 0) {
- return text;
- }
- }
- if (this.options.baseUrl && !originIndependentUrl.test(href)) {
- href = resolveUrl(this.options.baseUrl, href);
- }
- try {
- href = encodeURI(href).replace(/%25/g, '%');
- } catch (e) {
+ href = cleanUrl(this.options.sanitize, this.options.baseUrl, href);
+ if (href === null) {
  return text;
  }
  var out = '<a href="' + escape(href) + '"';
@@ -1063,9 +1047,11 @@ Renderer.prototype.link = function(href, title, text) {
 };
 
 Renderer.prototype.image = function(href, title, text) {
- if (this.options.baseUrl && !originIndependentUrl.test(href)) {
- href = resolveUrl(this.options.baseUrl, href);
+ href = cleanUrl(this.options.sanitize, this.options.baseUrl, href);
+ if (href === null) {
+ return text;
  }
+
  var out = '<img src="' + href + '" alt="' + text + '"';
  if (title) {
  out += ' title="' + title + '"';
@@ -1343,6 +1329,30 @@ function edit(regex, opt) {
  };
 }
 
+function cleanUrl(sanitize, base, href) {
+ if (sanitize) {
+ try {
+ var prot = decodeURIComponent(unescape(href))
+ .replace(/[^\w:]/g, '')
+ .toLowerCase();
+ } catch (e) {
+ return null;
+ }
+ if (prot.indexOf('javascript:') === 0 || prot.indexOf('vbscript:') === 0 || prot.indexOf('data:') === 0) {
+ return null;
+ }
+ }
+ if (base && !originIndependentUrl.test(href)) {
+ href = resolveUrl(base, href);
+ }
+ try {
+ href = encodeURI(href).replace(/%25/g, '%');
+ } catch (e) {
+ return null;
+ }
+ return href;
+}
+
 function resolveUrl(base, href) {
  if (!baseUrls[' ' + base]) {
  // we can ignore everything in base after the last slash of its path component,

diff --git a/test/new/images.html b/test/new/images.html
@@ -0,0 +1,5 @@
+<p>Image</p>
+<p>Image</p>
+<p>Image</p>
+<p>Image</p>
+<p>Image</p>
diff --git a/test/new/images.md b/test/new/images.md
@@ -0,0 +1,12 @@
+---
+sanitize: true
+---
+![Image](javascript:alert)
+
+![Image](vbscript:alert)
+
+![Image](javascript&colon;alert&#40;1&#41;)
+
+![Image](javascript&#58document;alert&#40;1&#41;)
+
+![Image](data:text/html;base64,PHNjcmlwdD5hbGVydCgnWFNTJyk8L3NjcmlwdD4K)