Always save uploads to a tmpdir first and cleanup afterwards

This makes sure no unintended files are permanently saved.

Co-authored-by: Yannick Bungers <git@innay.de>
Signed-off-by: David Mehren <git@herrmehren.de>

lib/web/imageRouter/filesystem.js

@@ -1,6 +1,7 @@
 'use strict'
 const URL = require('url').URL
 const path = require('path')
+const fs = require('fs')
 
 const config = require('../../config')
 const logger = require('../../logger')
@@ -16,5 +17,13 @@ exports.uploadImage = function (imagePath, callback) {
     return
   }
 
-  callback(null, (new URL(path.basename(imagePath), config.serverURL + '/uploads/')).href)
+  const fileName = path.basename(imagePath)
+  // move image from temporary path to upload directory
+  try {
+    fs.copyFileSync(imagePath, path.join(config.uploadsPath, fileName))
+  } catch (e) {
+    callback(new Error('Error while moving file'), null)
+    return
+  }
+  callback(null, (new URL(fileName, config.serverURL + '/uploads/')).href)
 }

lib/web/imageRouter/index.js

@@ -4,6 +4,9 @@ const Router = require('express').Router
 const formidable = require('formidable')
 const path = require('path')
 const FileType = require('file-type')
+const fs = require('fs')
+const os = require('os')
+const rimraf = require('rimraf')
 
 const config = require('../../config')
 const logger = require('../../logger')
@@ -30,25 +33,27 @@ async function checkUploadType (filePath) {
 
 // upload image
 imageRouter.post('/uploadimage', function (req, res) {
-  var form = new formidable.IncomingForm()
+  if (!req.isAuthenticated() && !config.allowAnonymous && !config.allowAnonymousEdits) {
-
+    logger.error(`Image upload error: Anonymous edits and therefore uploads are not allowed)`)
-  form.keepExtensions = true
+    return errors.errorForbidden(res)
-
-  if (config.imageUploadType === 'filesystem') {
-    form.uploadDir = config.uploadsPath
   }
 
+  var form = new formidable.IncomingForm()
+  form.keepExtensions = true
+  const tmpDir = fs.mkdtempSync(path.join(os.tmpdir(), 'hedgedoc-'))
+  form.uploadDir = tmpDir
+
   form.parse(req, async function (err, fields, files) {
     if (err) {
       logger.error(`Image upload error: formidable error: ${err}`)
-      return errors.errorForbidden(res)
+      rimraf(tmpDir)
-    } else if (!req.isAuthenticated() && !config.allowAnonymous && !config.allowAnonymousEdits) {
-      logger.error(`Image upload error: Anonymous edits and therefore uploads are not allowed)`)
       return errors.errorForbidden(res)
     } else if (!files.image || !files.image.path) {
       logger.error(`Image upload error: Upload didn't contain file)`)
+      rimraf.sync(tmpDir)
       return errors.errorBadRequest(res)
     } else if (!await checkUploadType(files.image.path)) {
+      rimraf.sync(tmpDir)
       return errors.errorBadRequest(res)
     } else {
       logger.debug(`SERVER received uploadimage: ${JSON.stringify(files.image)}`)
@@ -56,6 +61,7 @@ imageRouter.post('/uploadimage', function (req, res) {
       const uploadProvider = require('./' + config.imageUploadType)
       logger.debug(`imageRouter: Uploading ${files.image.path} using ${config.imageUploadType}`)
       uploadProvider.uploadImage(files.image.path, function (err, url) {
+        rimraf.sync(tmpDir)
         if (err !== null) {
           logger.error(err)
           return res.status(500).end('upload image error')

package.json

@@ -112,6 +112,7 @@
     "readline-sync": "^1.4.7",
     "request": "^2.88.0",
     "reveal.js": "^3.9.2",
+    "rimraf": "^3.0.2",
     "scrypt-async": "^2.0.1",
     "scrypt-kdf": "^2.0.1",
     "select2": "^3.5.2-browserify",