Browse Source

Try to fix some security issues.

Thanks to responsible security disclosure contributors and xet7 !
Lauri Ojansivu 2 years ago
parent
commit
ff993e7c91

+ 1 - 1
client/components/activities/activities.js

@@ -1,4 +1,4 @@
-import DOMPurify from 'dompurify';
+import DOMPurify from 'isomorphic-dompurify';
 import { TAPi18n } from '/imports/i18n';
 
 const activitiesPerPage = 500;

+ 10 - 10
client/components/cards/attachments.jade

@@ -34,22 +34,22 @@ template(name="attachmentsGalery")
   .attachments-galery
     each attachments
       .attachment-item
-        a.attachment-thumbnail.swipebox(href="{{link}}" title="{{name}}")
+        a.attachment-thumbnail.swipebox(href="{{link}}" title="{{sanitize name}}")
           if link
             if isImage
-              img.attachment-thumbnail-img(src="{{link}}")
+              img.attachment-thumbnail-img(src="{{link}}" title="{{sanitize name}}")
             else if($eq extension 'mp3')
-                video(width="100%" height="100%" controls="true")
-                  source(src="{{link}}" type="audio/mpeg")
+              video(width="100%" height="100%" title="{{sanitize name}}" controls="true")
+                source(src="{{link}}" type="audio/mpeg")
             else if($eq extension 'ogg')
-                video(width="100%" height="100%" controls="true")
-                  source(src="{{link}}" type="video/ogg")
+              video(width="100%" height="100%" title="{{sanitize name}}" controls="true")
+                source(src="{{link}}" type="video/ogg")
             else if($eq extension 'webm')
-                video(width="100%" height="100%" controls="true")
-                  source(src="{{link}}" type="video/webm")
+              video(width="100%" height="100%" title="{{sanitize name}}" controls="true")
+                source(src="{{link}}" type="video/webm")
             else if($eq extension 'mp4')
-                video(width="100%" height="100%" controls="true")
-                  source(src="{{link}}" type="video/mp4")
+              video(width="100%" height="100%" title="{{sanitize name}}" controls="true")
+                source(src="{{link}}" type="video/mp4")
             else
               span.attachment-thumbnail-ext= extension
         p.attachment-details

+ 8 - 0
client/components/cards/attachments.js

@@ -1,4 +1,5 @@
 import { ObjectID } from 'bson';
+import DOMPurify from 'isomorphic-dompurify';
 
 const filesize = require('filesize');
 const prettyMilliseconds = require('pretty-ms');
@@ -21,6 +22,9 @@ Template.attachmentsGalery.helpers({
     const ret = filesize(size);
     return ret;
   },
+  sanitize(value) {
+    return DOMPurify.sanitize(value);
+  },
 });
 
 Template.cardAttachmentsPopup.onCreated(function() {
@@ -49,6 +53,10 @@ Template.cardAttachmentsPopup.events({
       let uploads = [];
       for (const file of files) {
         const fileId = new ObjectID().toString();
+        // If filename is not same as sanitized filename, has XSS, then cancel upload
+        if (file.name !== DOMPurify.sanitize(file.name)) {
+          return false;
+        }
         const config = {
           file: file,
           fileId: fileId,

+ 1 - 1
client/components/main/editor.js

@@ -283,7 +283,7 @@ BlazeComponent.extendComponent({
   }
 }).register('editor');
 
-import DOMPurify from 'dompurify';
+import DOMPurify from 'isomorphic-dompurify';
 
 // Additional  safeAttrValue function to allow for other specific protocols
 // See https://github.com/leizongmin/js-xss/issues/52#issuecomment-241354114

+ 6 - 3
models/attachments.js

@@ -6,6 +6,7 @@ import fs from 'fs';
 import path from 'path';
 import { AttachmentStoreStrategyFilesystem, AttachmentStoreStrategyGridFs, AttachmentStoreStrategyS3 } from '/models/lib/attachmentStoreStrategy';
 import FileStoreStrategyFactory, {moveToStorage, rename, STORAGE_NAME_FILESYSTEM, STORAGE_NAME_GRIDFS, STORAGE_NAME_S3} from '/models/lib/fileStoreStrategy';
+import DOMPurify from 'isomorphic-dompurify';
 
 let attachmentUploadExternalProgram;
 let attachmentUploadMimeTypes = [];
@@ -149,9 +150,11 @@ if (Meteor.isServer) {
     renameAttachment(fileObjId, newName) {
       check(fileObjId, String);
       check(newName, String);
-
-      const fileObj = Attachments.findOne({_id: fileObjId});
-      rename(fileObj, newName, fileStoreStrategyFactory);
+      // If new name is same as sanitized name, does not have XSS, allow rename file
+      if (newName === DOMPurify.sanitize(newName)) {
+        const fileObj = Attachments.findOne({_id: fileObjId});
+        rename(fileObj, newName, fileStoreStrategyFactory);
+      }
     },
     validateAttachment(fileObjId) {
       check(fileObjId, String);

+ 1 - 0
models/users.js

@@ -2499,6 +2499,7 @@ if (Meteor.isServer) {
         Accounts.destroyToken(userId, token);
         data.message = 'Delete token: [' + token + '] from user: ' + userId;
       } else if (userId) {
+        check(userId, String);
         Users.update(
           {
             _id: userId,

File diff suppressed because it is too large
+ 553 - 249
package-lock.json


+ 1 - 0
package.json

@@ -41,6 +41,7 @@
     "filesize": "^8.0.7",
     "i18next": "^21.6.16",
     "i18next-sprintf-postprocessor": "^0.2.2",
+    "isomorphic-dompurify": "^1.0.0",
     "jquery": "^2.2.4",
     "jquery-ui": "^1.13.0",
     "jquery-ui-touch-punch": "^0.2.3",

+ 1 - 1
packages/markdown/src/template-integration.js

@@ -1,4 +1,4 @@
-import DOMPurify from 'dompurify';
+import DOMPurify from 'isomorphic-dompurify';
 
 var Markdown = require('markdown-it')({
   html: true,

Some files were not shown because too many files changed in this diff