Merge pull request from GHSA-xc2r-jf2x-gjr8

shubhamjain · web-flow · commit d3562fc08497 · 2023-08-12T09:56:16.000+05:30
Fix XSS vulnerability
diff --git a/package.json b/package.json
@@ -9,7 +9,7 @@
         "url": "https://github.com/shubhamjain/svg-loader.git"
     },
     "main": "dist/svg-loader.min.js",
-    "version": "1.6.8",
+    "version": "1.6.9",
     "scripts": {
         "postinstall": "npm-run-all build:*",
         "build:js": "cross-env NODE_ENV=production webpack build",
diff --git a/svg-loader.js b/svg-loader.js
@@ -62,6 +62,12 @@ const getAllEventNames = () => {
         }
     }
 
+    // SVG <animate> events
+    DOM_EVENTS.push('onbegin', 'onend', 'onrepeat');
+
+    // Some non-standard events, just in case the browser is handling them
+    DOM_EVENTS.push('onfocusin', 'onfocusout', 'onbounce', 'onfinish', 'onshow');
+
     return DOM_EVENTS;
 };
 
@@ -128,7 +134,7 @@ const renderBody = (elem, options, body) => {
             }
 
             // Remove "javascript:..." unless specifically enabled
-            if (["href", "xlink:href"].includes(name) && value.startsWith("javascript") && !enableJs) {
+            if (["href", "xlink:href", "values"].includes(name) && value.startsWith("javascript") && !enableJs) {
                 attributesToRemove.push(name);
             }
         }
@@ -391,4 +397,4 @@ globalThis.SVGLoader.destroyCache = async () => {
             localStorage.removeItem(key);
         }
     });
-}
+}
diff --git a/test/icons/svg-xss-2.svg b/test/icons/svg-xss-2.svg
@@ -0,0 +1,3 @@
+<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
+  <animate onbegin=alert(1) attributeName=x dur=1s>
+</svg>
diff --git a/test/icons/svg-xss.svg b/test/icons/svg-xss.svg
@@ -0,0 +1,3 @@
+    <svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
+        <animate xlink:href=#xss attributeName=href values=javascript:alert(1) /><a id=xss><text x=20 y=20>XSS</text></a></svg>
+    </svg>
diff --git a/test/index.html b/test/index.html
@@ -1,6 +1,5 @@
 <html>
     <head>
-        <link rel="stylesheet" href="/main.css">
         <style>
             body {
                 padding-bottom: 80px;
@@ -61,6 +60,15 @@ <h1>
     <div>This is a more complicated SVG with JS, only map with overlay message should be shown</div>
     <svg data-src="./USStates.svg"></svg>
 
+    <div class="heading">Icon with XSS</div>
+    <div>Tricky XSS that needs to be filtered out. On clicking "X" nothing should happen </div>
+    <svg data-src="/icons/svg-xss.svg"></svg>
+
+    <div class="heading">Icon with XSS 2</div>
+    <div>Alert should not come</div>
+    <svg data-src="/icons/svg-xss-2.svg"></svg>
+
+
     <div class="heading">Icon with JS (enabled)</div>
     <div>On hover alert should be thrown</div>
     <svg data-src="/icons/cog-with-script.svg" data-js="enabled"></svg>

Original file line number	Diff line number	Diff line change
`@@ -62,6 +62,12 @@ const getAllEventNames = () => {`
`62`	`62`	`}`
`63`	`63`	`}`
`64`	`64`
	`65`	`+ // SVG <animate> events`
	`66`	`+ DOM_EVENTS.push('onbegin', 'onend', 'onrepeat');`
	`67`	`+`
	`68`	`+ // Some non-standard events, just in case the browser is handling them`
	`69`	`+ DOM_EVENTS.push('onfocusin', 'onfocusout', 'onbounce', 'onfinish', 'onshow');`
	`70`	`+`
`65`	`71`	`return DOM_EVENTS;`
`66`	`72`	`};`
`67`	`73`
`@@ -128,7 +134,7 @@ const renderBody = (elem, options, body) => {`
`128`	`134`	`}`
`129`	`135`
`130`	`136`	`// Remove "javascript:..." unless specifically enabled`
`131`		`- if (["href", "xlink:href"].includes(name) && value.startsWith("javascript") && !enableJs) {`
	`137`	`+ if (["href", "xlink:href", "values"].includes(name) && value.startsWith("javascript") && !enableJs) {`
`132`	`138`	`attributesToRemove.push(name);`
`133`	`139`	`}`
`134`	`140`	`}`
`@@ -391,4 +397,4 @@ globalThis.SVGLoader.destroyCache = async () => {`
`391`	`397`	`localStorage.removeItem(key);`
`392`	`398`	`}`
`393`	`399`	`});`
`394`		`-}`
	`400`	`+}`