Skip to content

[Security] Defaults and example-code allow XSS-Attack #313

@macrozone

Description

@macrozone

If you follow the example-code in the readme, you are vulnerable to an XSS-Attack

(Code from readme):

 onBeforeUpload: function (file) {
    // Allow upload files under 10MB, and only in png/jpg/jpeg formats
    if (file.size <= 10485760 && /png|jpg|jpeg/i.test(file.extension)) {
      return true;
    } else {
      return 'Please upload image, with size equal or less than 10MB';
    }
  }

Here we check the extension of the file (on both client and server),
but for file delivery, the mime type that client provides is taken. You can therefore upload anything you want which will delivered to the browser with any mimetype that you want.

The following attack is possible:

  • create a html-file with nasty javascript that reads cookies and tokens
  • rename the file to .jpg
  • create crafted requests (e.g. with postman) which upload the file as usual, but override file.type with "text/html" in the first request.
  • send the resulting url to your victims which look like a jpg, but will be interpreted as html by the browser.

Solutions

  • do not rely on the mime-type from the client at all. Calculate the mime-type from the extension when the file is served (which is what nginx does, so i guess that's ok)
  • If we stick with the current approach, we should enforce best practices by changing the example code in the readme to:
// check mimetype that the user provides.
// this is crucial, because we use this mimetype when serving files.
// you can additionaly check the extension as well
const isImage = file => file.type.indexOf("image") === 0 &&   /png|jpg|jpeg/i.test(file.extension);

 onBeforeUpload: function (file) {
    // Allow upload files under 10MB, and only image formats
    if (file.size <= 10485760 && isImage(file)) {
      return true;
    } else {
      return 'Please upload image, with size equal or less than 10MB';
    }
  }
  • Maybe we can also provide an optional practice which will verify the mime-type by the content of a file

// if you need additional security,
// you can verify the mimetype by looking at the content of the file
// and delete it, if it looks malicious.
// E.g. you can use https://github.com/mscdex/mmmagic for this

const isImageMime = (mimeType) => mimeType.indexOf('image') === 0;
//...
onAfterUpload(file) {
    if (Meteor.isServer) {
      // check real mimetype
      const { Magic, MAGIC_MIME_TYPE } = require('mmmagic');
      const magic = new Magic(MAGIC_MIME_TYPE);
      magic.detectFile(file.path, Meteor.bindEnvironment((err, mimetype) => {
        if (err || !isImageMime(mimetype)) {
          // is not a real image --> delete
          console.log('onAfterUpload, not an image: ', file.path);
          console.log('deleted', file.path);
          Images.remove(file._id);
        }
      }));
    }
  },

Metadata

Metadata

Assignees

No one assigned

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions