multer icon indicating copy to clipboard operation
multer copied to clipboard

Multer doesn't filter mimetypes correctly

Open drewlearns opened this issue 2 years ago • 6 comments

const express = require("express");
const PhotosRouter = express.Router();
const db = require("../models");
const multer = require("multer");

const fileStorageEngine = multer.diskStorage({
  destination: (request, file, callback) => {
    callback(null, "./public/images");
  },
  filename: (request, file, callback) => {
    callback(null, Date.now() + "--" + file.originalname);
  },
});

const uploadFilter = function (request, file, callback) {
    
    const fileType = file.mimetype.split('/');
    
    if (fileType[0] === "image") {
      callback(null, true)
    }else{
      callback("You are trying to upload a file that is not an image. Go back and try again", false)
    }
};

const upload = multer({ 
  fileFilter: uploadFilter,
  storage: fileStorageEngine
});

PhotosRouter.route("/").get((request, response) => {
  db.photo
    .findAll()
    .then((photos) => {
      console.log("GET IMAGES");
      response.redirect("/");
    })
    .catch((error) => {
      response.send(error);
    });
});

PhotosRouter.route("/")
.post(upload.single("photo"), (request, response) => {
    const title = request.body.title;
    const mediaLocation = request.file.filename;
    db.photo
      .create({ title: title, mediaLocation: mediaLocation })
      .then((photo) => {
        console.log("POST IMAGES");
        // response.send(photo);
        response.redirect('/');
      })
      .catch((error) => {
        response.send(error);
      });
  })

  .put((request, response) => {
    console.log("PUT IMAGES");
    response.send("PUT IMAGES");
  })
  .delete((request, response) => {
    console.log("DELETE IMAGES");
    response.send("DELETE IMAGES");
  });

PhotosRouter.route("/:id") // for removing photos
  .delete((request, response) => {
    const id = request.params.id;
    db.photo
      .destroy({ where: { id: id } })
      .then((photo) => {
        response.send("Deleted");
      })
      .catch((error) => {
        response.send(error);
      });
  });

  module.exports = PhotosRouter;

I've been trying to make sure that if someone uploaded say a shell script that was renamed from script.sh to script.png that the file would not upload but no matter what I've tried, multer still allows this file to be uploaded. Am I missing something?

drewlearns avatar Nov 24 '22 13:11 drewlearns

The solution I came up with was on the get route for the home page, everytime it's loaded it checks using bash for bad files

PageRouter.get("/", (request, response) => {
  if (request.session.userId) {
    const { exec } = require("child_process");
    exec(
      `for item in $(ls $(pwd)/public/images); do
      if [ $( file --mime-type $(pwd)/public/images/$item -b ) != "image/jpeg" ] && [ $( file --mime-type $(pwd)/public/images/$item -b ) != "image/png" ]; then
      echo "$(pwd)/public/images/$item"
      fi; 
      done;`,
      (error, stdout, stderr) => {
        if (stdout) {
          fs.unlink(stdout.slice(0, -1), (err) => {
            if (err) {
              throw err;
            }
          });
          console.log(`Deleted ${stdout} because it wasn't an image`);
        }
      }
    );
    ```
    
    It's less than ideal but it works.

drewlearns avatar Nov 25 '22 16:11 drewlearns

use multer middleware like this

const multer = require('multer');
const path = require('path');
const fs = require('fs')
require('dotenv').config({ path: path.resolve(__dirname, '../.env') });
const crypto = require('crypto');

const contentImage={
    storage:function(){
        var storage = multer.diskStorage({
          destination: function(req, file, cb) {
            const filesDir = process.env.SINGLE_IMAGE_DIRECTORY
            if (!fs.existsSync(filesDir)) {
              fs.mkdirSync(filesDir, { recursive: true })
            }
            cb(null, filesDir);
          },
        filename: function (req, file, cb) {
          let buf = crypto.randomBytes(16);
          buf = buf.toString('hex'); 
          cb(null,buf+path.extname(file.originalname));
        }
      })
      
      return storage;
},
allowedFiles:function(req, file, cb) {
    // Accept images only
    if (!file.originalname.match(/\.(jpg|JPG|jpeg|JPEG|png|PNG|gif|GIF|webp|WEBP)$/)) {
        req.fileValidationError = 'Only jpg|JPG|jpeg|JPEG|png|PNG|gif|GIF|webp|WEBP file type are allowed!';
        return cb(new Error('Only jpg|JPG|jpeg|JPEG|png|PNG|gif|GIF|webp|WEBP file type  are allowed!'), false);
    }
    return cb(null, true);
}
}

const uploadSingleImage = multer({
  storage:contentImage.storage(),
  fileFilter:contentImage.allowedFiles,
  limits: {
      fileSize: 1024*1024*5
    },
}).single("singleImage");

module.exports={uploadSingleImage}

and function in controller like this

const { uploadSingleImage } = require("../middleware/upload.single.image");
const uploadSingleImagee = async (req, res, next) => {
  uploadSingleImage(req, res, function (err) {
      if (err instanceof multer.MulterError) {
        if (err.code === "LIMIT_UNEXPECTED_FILE") {
          return res.status(400).json({ message: "Too many files to upload." });
        }
        return res.status(400).json({ message: err.message });
      } else if (err) {
        return res.status(400).json({ message: err.message });
      } else if (!req.file) {
        return res.status(400).json({ message: "File is required!" });
      } else {
        res.status(200).json({message:"File Uploaded Successfully", ...req.file});
        next();
      }
    })
};
module.exports={uploadSingleImagee}

and of course route like this

const control = require('../controller/aep.upload.controller')
router.post('/uploadsingleimage', control.uploadSingleImagee);

alamjamal avatar Nov 29 '22 05:11 alamjamal

Problem

The reason you're getting incorrect result might be due to multer or busboy rely solely on file extensions to determine a file's MIME type because file extensions can be easily changed or spoofed. Attackers can modify the extension of a file to make it appear as a different file type, even if it contains malicious content. This technique is known as file extension spoofing or file extension hiding.

Solutions

To accurately determine a file's MIME type, it is essential to inspect the file's contents, usually by examining its header, signature, or magic number. The header or signature is a sequence of bytes at the beginning of a file that identifies its format and helps determine its MIME type.

Code Setup

Details
 import express from "express";
 import multer from "multer";
 import fs from "fs";

 const upload = multer({ dest: "uploads/" }).array("attachments", 5);

 const app = express();

 app.post(
     "/api/upload",
     upload,
     fileTypeFilter({ discard_unknown_type: true, discard_mismatch_mime: true }),
     (req, res) => {
         const { files, _fTypeErrors } = req;
         res.json({ errors: _fTypeErrors, files });
     }
 );

 function _drop_file(req, file) {
     req.files = req.files.filter((_f) => _f.originalname !== file.originalname);
     fs.unlink(file.path, (e) => {});
 }

 app.listen(3000);

NOTE: discard_unknown_type is only required for file-type library because it does not support all MIME types, for more read solution 2's last part.

Solution 1 (Most Accurate Result)

  • Source: https://github.com/anasshakil/metadata
Details

Code

 npm i @enviro/metadata
 import Metadata from "@enviro/metadata";
 
 function fileTypeFilter(options = {}) {
     const { discard_mismatch_mime } = options;
     return async (req, res, next) => {
         req._fTypeErrors = [];
         for (const file of req.files) {
             const { mimetype, path: relativePath, originalname } = file;
             const metadata = await Metadata.get(relativePath, {
                 path: "path/to/Exiftool",
             });
             const fileType = metadata[0].MIMEType;
             if (mimetype !== fileType && discard_mismatch_mime) {
                 req._fTypeErrors.push(
                     `${originalname} has a mismatched file type!`
                 );
                 _drop_file(req, file);
             }
         }
         return next();
     };
 }

Solution 2 (Checking Magic Number)

  • Source: https://github.com/sindresorhus/file-type
Details

Code

 npm i file-type
 import { fileTypeFromFile } from "file-type";
 
 function fileTypeFilter(options = {}) {
     const { discard_unknown_type, discard_mismatch_mime } = options;
     return async (req, res, next) => {
         req._fTypeErrors = [];
         for (const file of req.files) {
             const { mimetype, path: relativePath, originalname } = file;
             const fileType = await fileTypeFromFile(relativePath);
             if (!fileType && discard_unknown_type) {
                 req._fTypeErrors.push(
                     `${originalname} has an unsupported file type!`
                 );
                 _drop_file(req, file);
             } else if (mimetype !== fileType.mime && discard_mismatch_mime) {
                 req._fTypeErrors.push(
                     `${originalname} has a mismatched file type!`
                 );
                 _drop_file(req, file);
             }
         }
         return next();
     };
 }

⚠️ NOTE: This solution will not work for text-based formats like .txt, .csv, .svg, etc.

anasshakil avatar Mar 01 '23 09:03 anasshakil

Unfortunately, multer (and other similar libraries) primarily uses the MIME type that is sent with the upload request, and this MIME type is usually determined by the file extension.

If someone renames a shell script to have a .png extension, the operating system will send it with a MIME type of image/png, and multer will accept it based on that MIME type, even though the actual content of the file is not a valid image.

const uploadFilter = function (request, file, cb) {
    if (!file.originalname.match(/\.(jpg|JPG|jpeg|JPEG|png|PNG|gif|GIF)$/)) {
        req.fileValidationError = 'Only image files are allowed!';
        return cb(new Error('Only image files are allowed!'), false);
    } ...
const upload = multer({
  fileFilter: uploadFilter,
  storage: fileStorageEngine,
});

Use like this, it should work perfectly fine.

sunanda35 avatar Jun 05 '23 22:06 sunanda35