All: Security: Prevent XSS by sanitizing certain HTML attributes

laurent22 · May 19, 2023 · 9e90d90 · 9e90d90
1 parent ccec93e
commit 9e90d90
Show file tree

Hide file tree

Showing 4 changed files with 14 additions and 12 deletions.
diff --git a/packages/app-cli/tests/md_to_html/sanitize_15.html b/packages/app-cli/tests/md_to_html/sanitize_15.html
@@ -1 +1 @@
-<use href="data:image/svg+xml,&lt;svg id=&apos;x&apos; xmlns=&apos;http://www.w3.org/2000/svg&apos;&gt;&lt;image href=&apos;asdf&apos; onerror=&apos;top.require(`child_process`).execSync(`calc.exe`)&apos; /&gt;&lt;/svg&gt;#x" class="jop-noMdConv">
+<use href="#" class="jop-noMdConv">
diff --git a/packages/app-cli/tests/md_to_html/sanitize_16.html b/packages/app-cli/tests/md_to_html/sanitize_16.html
@@ -0,0 +1 @@
+<map name="test" class="jop-noMdConv"><area coords="0,0,1000,1000" href="#" class="jop-noMdConv"/></map><img usemap="#test" src="https://github.com/Ry0taK.png" class="jop-noMdConv"/>
diff --git a/packages/app-cli/tests/md_to_html/sanitize_16.md b/packages/app-cli/tests/md_to_html/sanitize_16.md
@@ -0,0 +1 @@
+<map name="test"><area coords="0,0,1000,1000" href="javascript:top.require(`child_process`).execSync(`calc.exe`)"></map><img usemap="#test" src="https://github.com/Ry0taK.png">
diff --git a/packages/renderer/htmlUtils.ts b/packages/renderer/htmlUtils.ts
@@ -233,18 +233,18 @@ class HtmlUtils {
 					delete attrs[attrName];
 				}
 
-				if (name === 'a') {
-					// Make sure that only non-acceptable URLs are filtered out.
-					// In particular we want to exclude `javascript:` URLs.
-					if ('href' in attrs && !this.isAcceptedUrl(attrs['href'])) {
-						attrs['href'] = '#';
-					}
+				// Make sure that only non-acceptable URLs are filtered out. In
+				// particular we want to exclude `javascript:` URLs. This
+				// applies to A tags, and also AREA ones but to be safe we don't
+				// filter on the tag name and process all HREF attributes.
+				if ('href' in attrs && !this.isAcceptedUrl(attrs['href'])) {
+					attrs['href'] = '#';
+				}
 
-					// We need to clear any such attribute, otherwise it will
-					// make any arbitrary link open within the application.
-					if ('data-from-md' in attrs) {
-						delete attrs['data-from-md'];
-					}
+				// We need to clear any such attribute, otherwise it will
+				// make any arbitrary link open within the application.
+				if ('data-from-md' in attrs) {
+					delete attrs['data-from-md'];
 				}
 
 				if (options.addNoMdConvClass) {