Added sanitization for svg uploads (#20264)

ref https://linear.app/tryghost/issue/ENG-856
- svgs were not previously sanitized and could contain scripts

ghost/core/core/server/web/api/middleware/upload.js

@@ -6,6 +6,7 @@ const errors = require('@tryghost/errors');
 const config = require('../../../../shared/config');
 const tpl = require('@tryghost/tpl');
 const logging = require('@tryghost/logging');
+const {JSDOM} = require('jsdom');
 
 const messages = {
     db: {
@@ -144,14 +145,33 @@ const checkFileExists = (fileData) => {
 
 const checkFileIsValid = (fileData, types, extensions) => {
     const type = fileData.mimetype;
-
     if (types.includes(type) && extensions.includes(fileData.ext)) {
         return true;
     }
-
     return false;
 };
 
+/**
+ * 
+ * @param {String} filepath 
+ * @returns {Boolean}
+ * 
+ * Checks for the presence of <script> tags or 'on' attributes in an SVG file
+ * 
+ */
+const isSvgSafe = (filepath) => {
+    const fileContent = fs.readFileSync(filepath, 'utf8');
+    const document = new JSDOM(fileContent).window.document;
+    document.body.innerHTML = fileContent;
+    const svgEl = document.body.firstElementChild;
+    
+    const attributes = Array.from(svgEl.attributes).map(({name}) => name);
+    const hasScriptAttr = !!attributes.find(attr => attr.startsWith('on'));
+    const scripts = svgEl.getElementsByTagName('script');
+
+    return scripts.length === 0 && !hasScriptAttr ? true : false;
+};
+
 /**
  *
  * @param {Object} options
@@ -190,6 +210,14 @@ const validation = function ({type}) {
             }));
         }
 
+        if (req.file.ext === '.svg') {
+            if (!isSvgSafe(req.file.path)) {
+                return next(new errors.UnsupportedMediaTypeError({
+                    message: 'SVG files cannot contain <script> tags or "on" attributes.'
+                }));
+            }
+        }
+
         next();
     };
 };
@@ -261,5 +289,6 @@ module.exports = {
 // Exports for testing only
 module.exports._test = {
     checkFileExists,
-    checkFileIsValid
+    checkFileIsValid,
+    isSvgSafe
 };

ghost/core/test/unit/server/web/api/middleware/upload.test.js

@@ -1,18 +1,21 @@
 const should = require('should');
 const validation = require('../../../../../../core/server/web/api/middleware/upload')._test;
+const imageFixturePath = ('../../../../../utils/fixtures/images/');
+const fs = require('fs');
+const path = require('path');
 
 describe('web utils', function () {
     describe('checkFileExists', function () {
         it('should return true if file exists in input', function () {
-            validation.checkFileExists({mimetype: 'file', path: 'path'}).should.be.true();
+            validation.checkFileExists({mimetype: 'file', path: 'path'}).should.be.true;
         });
 
         it('should return false if file does not exist in input', function () {
-            validation.checkFileExists({}).should.be.false();
+            validation.checkFileExists({}).should.be.false;
         });
 
         it('should return false if file is incorrectly structured', function () {
-            validation.checkFileExists({type: 'file'}).should.be.false();
+            validation.checkFileExists({type: 'file'}).should.be.false;
         });
     });
 
@@ -22,22 +25,43 @@ describe('web utils', function () {
                 name: 'test.txt',
                 mimetype: 'text',
                 ext: '.txt'
-            }, ['text'], ['.txt']).should.be.true();
+            }, ['text'], ['.txt']).should.be.true;
 
             validation.checkFileIsValid({
                 name: 'test.jpg',
                 mimetype: 'jpeg',
                 ext: '.jpg'
-            }, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true();
+            }, ['text', 'jpeg'], ['.txt', '.jpg']).should.be.true;
         });
 
         it('returns false if file has invalid extension', function () {
-            validation.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).should.be.false();
+            validation.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['text'], ['.tar']).should.be.false;
-            validation.checkFileIsValid({name: 'test', mimetype: 'text'}, ['text'], ['.txt']).should.be.false();
+            validation.checkFileIsValid({name: 'test', mimetype: 'text'}, ['text'], ['.txt']).should.be.false;
         });
 
         it('returns false if file has invalid type', function () {
-            validation.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['archive'], ['.txt']).should.be.false();
+            validation.checkFileIsValid({name: 'test.txt', mimetype: 'text'}, ['archive'], ['.txt']).should.be.false;
+        });
+    });
+
+    describe('isSvgSafe', function () {
+        it('detects a <script> tag in a svg file', async function () {
+            const filepath = path.join(__dirname, imageFixturePath, 'svg-with-script.svg');
+            const dirtySvgContent = fs.readFileSync(filepath, 'utf8');
+            dirtySvgContent.should.containEql('<script');
+            validation.isSvgSafe(filepath).should.be.false;
+        });
+        it('detects a on attribute in a svg file', async function () {
+            const filepath = path.join(__dirname, imageFixturePath, 'svg-with-script2.svg');
+            const dirtySvgContent = fs.readFileSync(filepath, 'utf8');
+            dirtySvgContent.should.containEql('onclick');
+            validation.isSvgSafe(filepath).should.be.false;
+        });
+        it('returns true for a safe svg file', async function () {
+            const filepath = path.join(__dirname, imageFixturePath, 'ghost-logo.svg');
+            const dirtySvgContent = fs.readFileSync(filepath, 'utf8');
+            dirtySvgContent.should.not.containEql('<script');
+            validation.isSvgSafe(filepath).should.be.true;
         });
     });
 });

ghost/core/test/utils/fixtures/images/svg-with-script.svg

@@ -0,0 +1,8 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
+   <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:3;stroke:rgb(0,0,0)" />
+   <script type="text/javascript">
+      alert(1);
+   </script>
+</svg>

ghost/core/test/utils/fixtures/images/svg-with-script2.svg

@@ -0,0 +1,6 @@
+<?xml version="1.0" standalone="no"?>
+<!DOCTYPE svg PUBLIC "-//W3C//DTD SVG 1.1//EN" "http://www.w3.org/Graphics/SVG/1.1/DTD/svg11.dtd">
+<svg version="1.1" baseProfile="full" xmlns="http://www.w3.org/2000/svg">
+   <rect width="300" height="100" style="fill:rgb(0,0,255);stroke-width:3;stroke:rgb(0,0,0)" />
+   <button onclick="alert(1)">Click me</button>
+</svg>