CVE-2021-25978: Improper Neutralization of Input During Web Page Generation ('Cross-site Scripting') (CWE-79)
value: Apostrophe CMS versions between 2.63.0 to 3.3.1 are vulnerable to Stored XSS where an editor uploads an SVG file that contains malicious JavaScript onto the Images module, which triggers XSS once viewed.
CWE description: The product does not neutralize or incorrectly neutralizes user-controllable input before it is placed in output that is used as a web page that is served to other users.
CVSS v3 base score: 5.4
Published: 2021-11-07T18:15Z
Repository: https://github.com/apostrophecms/apostrophe
Language: JSON
Files changed: CHANGELOG.md, modules/@apostrophecms/attachment/index.js, modules/@apostrophecms/i18n/i18n/en.json, package.json
Commit date: 2021-09-07 16:14:18 -0400
Commit message: sanitize SVG uploads, including previous uploads

Diff: diff --git a/CHANGELOG.md b/CHANGELOG.md
index edba22b16..10a515076 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -1,7 +1,13 @@
 # Changelog
 
+## 3.3.2 - 2021-09-07
+
+### Security
+
+Users with permission to upload SVG files were previously able to do so even if they contained XSS attacks. In Apostrophe 3.x, the general public so far never has access to upload SVG files, so the risk is minor but could be used to phish access from an admin user uploading a file. While Apostrophe typically displays SVG files using the `img` tag, which ignores XSS vectors, an XSS attack could still be carried out if the image were opened directly via the Apostrophe media library's convenience link for doing so. Beginning in version 3.3.2 all SVG uploads are sanitized via DOMPurify to remove XSS attack vectors. In addition, all existing SVG attachments not already validated are passed through DOMPurify during a migration upon deployment of version 3.3.2. 
+
 ## 3.3.1 - 2021-09-01
 
 ### Fixes
 
 * In some situations it was possible for a relationship with just one selected document to list that document several times in the returned result, resulting in very large responses.
diff --git a/modules/@apostrophecms/attachment/index.js b/modules/@apostrophecms/attachment/index.js
index 87a20eb71..e0bd1a2a2 100644
--- a/modules/@apostrophecms/attachment/index.js
+++ b/modules/@apostrophecms/attachment/index.js
@@ -1,9 +1,11 @@
 const _ = require('lodash');
 const path = require('path');
 const fs = require('fs');
 const Promise = require('bluebird');
+const createDOMPurify = require('dompurify');
+const { JSDOM } = require('jsdom');
 
 module.exports = {
   options: { alias: 'attachment' },
   cascades: [ 'imageSizes' ],
   imageSizes: {
@@ -106,10 +108,11 @@ module.exports = {
 
     self.db = await self.apos.db.collection('aposAttachments');
     await self.db.createIndex({ docIds: 1 });
     await self.db.createIndex({ archivedDocIds: 1 });
     self.addLegacyMigrations();
+    self.addSvgSanitizationMigration();
   },
 
   tasks(self) {
     return {
       rescale: {
@@ -400,10 +403,20 @@ module.exports = {
             throw self.apos.error('forbidden');
           }
         }
         info.length = await self.apos.util.fileLength(file.path);
         info.md5 = await self.apos.util.md5File(file.path);
+        if (info.extension === 'svg') {
+          try {
+            await self.sanitizeSvg(file, info);
+          } catch (e) {
+            // Currently DOMPurify passes invalid SVG content without comment as long
+            // as it's not an SVG XSS attack vector, but make provision to report
+            // a relevant error if that changes
+            throw self.apos.error('invalid', req.t('apostrophe:fileInvalid'));
+          }
+        }
         if (self.isSized(extension)) {
           // For images we correct automatically for common file extension mistakes
           const result = await Promise.promisify(self.uploadfs.copyImageIn)(file.path, '/attachments/' + info._id + '-' + info.name, { sizes: self.imageSizes });
           info.extension = result.extension;
           info.width = result.width;
@@ -421,10 +434,24 @@ module.exports = {
         }
         info.createdAt = new Date();
         await self.db.insertOne(info);
         return info;
       },
+      // Given a path to a local svg file, sanitize any XSS attack vectors that
+      // may be present in the file. The caller is responsible for catching any
+      // exception thrown and treating that as an invalid file but there is no
+      // guarantee that invalid SVG files will be detected or cleaned up, only
+      // XSS attacks.
+      async sanitizeSvg(path) {
+        const readFile = require('util').promisify(fs.readFile);
+        const writeFile = require('util').promisify(fs.writeFile);
+        const window = new JSDOM('').window;
+        const DOMPurify = createDOMPurify(window);
+        const dirty = await readFile(path);
+        const clean = DOMPurify.sanitize(dirty);
+        return writeFile(path, clean);
+      },
       getFileGroup(extension) {
         return _.find(self.fileGroups, function (group) {
           const candidate = group.extensionMaps[extension] || extension;
           if (_.includes(group.extensions, candidate)) {
             return true;
@@ -705,11 +732,14 @@ module.exports = {
         // tasks like @apostrophecms/attachment:rescale. We need a robust solution that
         // does not require keeping a MongoDB cursor open too long. -Tom
         const batchSize = 100;
         let lastId = '';
         while (true) {
-          const docs = await self.db.find({ _id: { $gt: lastId } }).limit(batchSize).sort({ _id: 1 }).toArray();
+          const docs = await self.db.find({
+            ...(criteria || {}),
+            _id: { $gt: lastId }
+          }).limit(batchSize).sort({ _id: 1 }).toArray();
           if (!docs.length) {
             return;
           }
           await Promise.map(docs, each, { concurrency: limit });
           lastId = docs[docs.length - 1]._id;
@@ -1069,10 +1099,42 @@ module.exports = {
             return;
           }
           return bulk.execute();
         }
       },
+      async addSvgSanitizationMigration() {
+        self.apos.migration.add('svg-sanitization', async () => {
+          return self.each({
+            extension: 'svg',
+            sanitized: {
+              $ne: true
+            },
+          }, 1, async attachment => {
+            const tempFile = self.uploadfs.getTempPath() + '/' + self.apos.util.generateId() + '.' + attachment.extension;
+            const copyIn = require('util').promisify(self.uploadfs.copyIn);
+            const copyOut = require('util').promisify(self.uploadfs.copyOut);
+            const uploadfsPath = self.url(attachment, { uploadfsPath: true });
+            try {
+              await copyOut(uploadfsPath, tempFile);
+              await self.sanitizeSvg(tempFile);
+              await copyIn(tempFile, uploadfsPath);
+              await self.db.update({
+                _id: attachment._id
+              }, {
+                $set: {
+                  sanitized: true
+                }
+              });
+            } catch (e) {
+              console.error(e);
+              // This condition shouldn't occur, but do warn the operator if it does
+              // (possibly on input that is not really an SVG file at all)
+              self.apos.util.error(`Warning: unable to sanitize SVG file ${uploadfsPath}`);
+            }
+          });
+        });
+      },
       ...require('./lib/legacy-migrations')(self)
     };
   },
   helpers: [
     'url',
diff --git a/modules/@apostrophecms/i18n/i18n/en.json b/modules/@apostrophecms/i18n/i18n/en.json
index 9e177881f..d876d4995 100644
--- a/modules/@apostrophecms/i18n/i18n/en.json
+++ b/modules/@apostrophecms/i18n/i18n/en.json
@@ -126,10 +126,11 @@
   "fieldHasUnpublishedChanges": "This field has unpublished changes",
   "file": "File",
   "fileTag": "File Tag",
   "fileTags": "File Tags",
   "fileTypeNotAccepted": "File type was not accepted. Allowed extensions: {{ extensions }}",
+  "fileInvalid": "File was not accepted. It may be corrupted or its content may not match its file extension.",
   "files": "Files",
   "filter": "Filter",
   "filterByTag": "Filter by Tag",
   "filterResultsByType": "Filter Results By Type",
   "findOrAddTag": "Find an existing tag or add a new one",
diff --git a/package.json b/package.json
index 66bf2f6ea..5355ed929 100644
--- a/package.json
+++ b/package.json
@@ -55,10 +55,11 @@
     "css-loader": "^5.2.4",
     "cuid": "^1.3.8",
     "dayjs": "^1.9.8",
     "debounce-async": "0.0.2",
     "deep-get-set": "^1.1.1",
+    "dompurify": "^2.3.1",
     "eslint-plugin-promise": "^5.1.0",
     "express": "^4.16.4",
     "express-bearer-token": "^2.4.0",
     "express-session": "^1.17.1",
     "form-data": "^4.0.0",
@@ -68,10 +69,11 @@
     "html-to-text": "^5.1.1",
     "i18next": "^20.3.2",
     "i18next-http-middleware": "^3.1.4",
     "import-fresh": "^3.3.0",
     "is-wsl": "^2.2.0",
+    "jsdom": "^17.0.0",
     "klona": "^2.0.4",
     "launder": "^1.4.0",
     "lodash": "^4.17.20",
     "mini-css-extract-plugin": "^1.6.0",
     "minimatch": "^3.0.4",

Vulnerable code:           const docs = await self.db.find({ _id: { $gt: lastId } }).limit(batchSize).sort({ _id: 1 }).toArray();
Fixed code: ## 3.3.2 - 2021-09-07

### Security

Users with permission to upload SVG files were previously able to do so even if they contained XSS attacks. In Apostrophe 3.x, the general public so far never has access to upload SVG files, so the risk is minor but could be used to phish access from an admin user uploading a file. While Apostrophe typically displays SVG files using the `img` tag, which ignores XSS vectors, an XSS attack could still be carried out if the image were opened directly via the Apostrophe media library's convenience link for doing so. Beginning in version 3.3.2 all SVG uploads are sanitized via DOMPurify to remove XSS attack vectors. In addition, all existing SVG attachments not already validated are passed through DOMPurify during a migration upon deployment of version 3.3.2. 

const createDOMPurify = require('dompurify');
const { JSDOM } = require('jsdom');
    self.addSvgSanitizationMigration();
        if (info.extension === 'svg') {
          try {
            await self.sanitizeSvg(file, info);
          } catch (e) {
            // Currently DOMPurify passes invalid SVG content without comment as long
            // as it's not an SVG XSS attack vector, but make provision to report
            // a relevant error if that changes
            throw self.apos.error('invalid', req.t('apostrophe:fileInvalid'));
          }
        }
      // Given a path to a local svg file, sanitize any XSS attack vectors that
      // may be present in the file. The caller is responsible for catching any
      // exception thrown and treating that as an invalid file but there is no
      // guarantee that invalid SVG files will be detected or cleaned up, only
      // XSS attacks.
      async sanitizeSvg(path) {
        const readFile = require('util').promisify(fs.readFile);
        const writeFile = require('util').promisify(fs.writeFile);
        const window = new JSDOM('').window;
        const DOMPurify = createDOMPurify(window);
        const dirty = await readFile(path);
        const clean = DOMPurify.sanitize(dirty);
        return writeFile(path, clean);
      },
          const docs = await self.db.find({
            ...(criteria || {}),
            _id: { $gt: lastId }
          }).limit(batchSize).sort({ _id: 1 }).toArray();
      async addSvgSanitizationMigration() {
        self.apos.migration.add('svg-sanitization', async () => {
          return self.each({
            extension: 'svg',
            sanitized: {
              $ne: true
            },
          }, 1, async attachment => {
            const tempFile = self.uploadfs.getTempPath() + '/' + self.apos.util.generateId() + '.' + attachment.extension;
            const copyIn = require('util').promisify(self.uploadfs.copyIn);
            const copyOut = require('util').promisify(self.uploadfs.copyOut);
            const uploadfsPath = self.url(attachment, { uploadfsPath: true });
            try {
              await copyOut(uploadfsPath, tempFile);
              await self.sanitizeSvg(tempFile);
              await copyIn(tempFile, uploadfsPath);
              await self.db.update({
                _id: attachment._id
              }, {
                $set: {
                  sanitized: true
                }
              });
            } catch (e) {
              console.error(e);
              // This condition shouldn't occur, but do warn the operator if it does
              // (possibly on input that is not really an SVG file at all)
              self.apos.util.error(`Warning: unable to sanitize SVG file ${uploadfsPath}`);
            }
          });
        });
      },
  "fileInvalid": "File was not accepted. It may be corrupted or its content may not match its file extension.",
    "dompurify": "^2.3.1",
    "jsdom": "^17.0.0",