Insecure File Upload Vulnerability

[NinjaGPT]

Summary

During a comprehensive security assessment of CodiMD's open-source codebase, ZAST.AI identified an insecure file upload vulnerability affecting all versions. Notable implementation differences exist between higher versions (v2.5.4 - v2.2.1) and lower versions (≤v2.2.0), resulting in distinct security defects and exploitation vectors, under specific conditions, allows attackers to upload malicious files, potentially creating attack vectors for stored Cross-Site Scripting (XSS).

Details

High-Version (v2.5.4 - v2.2.1)

• Tested Version

v2.5.4

https://github.com/hackmdio/codimd/tree/f00df5058dd8ff70e057b8f4500714f7f773990a

---

Code Analysis

• lib/imageRouter/index.js

'use strict'

const fs = require('fs')
const path = require('path')
const Router = require('express').Router
const formidable = require('formidable')

const readChunk = require('read-chunk')
const imageType = require('image-type')
const mime = require('mime-types')

const config = require('../config')
const logger = require('../logger')
const response = require('../response')

const imageRouter = module.exports = Router()

function checkImageValid (filepath) {
  try {
    const buffer = readChunk.sync(filepath, 0, 12)
    /** @type {{ ext: string, mime: string } | null} */
    const mimetypeFromBuf = imageType(buffer)
    const mimeTypeFromExt = mime.lookup(path.extname(filepath))

    return mimetypeFromBuf && config.allowedUploadMimeTypes.includes(mimetypeFromBuf.mime) &&
          mimeTypeFromExt && config.allowedUploadMimeTypes.includes(mimeTypeFromExt)
  } catch (err) {
    logger.error(err)
    return false
  }
}

// upload image
imageRouter.post('/uploadimage', function (req, res) {
  var form = new formidable.IncomingForm({
    keepExtensions: true
  })

  form.parse(req, function (err, fields, files) {
    if (err || !files.image || !files.image.filepath) {
      response.errorForbidden(req, res)
    } else {
      if (config.debug) {
        logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image))
      }

      if (!checkImageValid(files.image.filepath)) {
        return response.errorForbidden(req, res)
      }

      const uploadProvider = require('./' + config.imageUploadType)
      uploadProvider.uploadImage(files.image.filepath, function (err, url) {
        // remove temporary upload file, and ignore any error
        fs.unlink(files.image.filepath, () => {})
        if (err !== null) {
          logger.error(err)
          return res.status(500).end('upload image error')
        }
        res.send({
          link: url
        })
      })
    }
  })
})

As evidenced, the application adopts the Formidable module for parsing and processing file uploads, with the keepExtensions parameter configured to true, thereby preserving the original file extensions. The checkImageValid method executes validation logic to verify file legitimacy, specifically designed to prevent attackers from circumventing security checks through the falsification of file Magic Bytes (file header signature values) and file extensions.

• lib\config\index.js

switch (config.imageUploadType) {
  case 'imgur':
    config.allowedUploadMimeTypes = [
      'image/jpeg',
      'image/png',
      'image/jpg',
      'image/gif'
    ]
    break
  default:
    config.allowedUploadMimeTypes = [
      'image/jpeg',
      'image/png',
      'image/jpg',
      'image/gif',
      'image/svg+xml',
      'image/bmp',
      'image/tiff'
    ]
}

From the above MIME allowlist for file uploads, it can be observed that SVG files are allowed, which represents one of the frequently exploited XSS (Cross-Site Scripting) attack vectors.

---

Test Procedure & Proof Of Concept

• User authentication required.

We conducted testing on the project's official demo site, which has deployed the latest version (v2.5.4) of the code. The vulnerable endpoint is https://demo.hedgedoc.org/uploadimage. We initiated Burp Suite and logged into the target demo website using its built-in browser. After creating a "New Note" and clicking "Upload Image" to upload a standard image file, Burp Suite captured the request. We then edited and replayed this request using the Repeater module, as shown in the figure.

In Burp Suite's Repeater, we discovered that the URL of the uploaded file is returned in the HTTP response: https://s3.hedgedoc.org/hd1-demo/uploads/df73c2eb-c757-4dcc-8c7d-91cbad556ec1.svg. Upon accessing this URL in the browser, the XSS vector was successfully executed:

Additionally, it is noteworthy that in the CSP policy defined in codimd\lib\csp.js, security restrictions are applied to same-origin inline JavaScript. This will be elaborated in detail in the analysis of lower versions below. However, in this particular case, the official demo site uploads files to a remote S3 server, thereby circumventing this CSP restriction.

---

Low-Version (≤ v2.2.0)

• Tested Version

v2.2.0

3e75445

---

Code Analysis

• codimd\imageRouter\index.js

'use strict'

const fs = require('fs')
const Router = require('express').Router
const formidable = require('formidable')

const config = require('../config')
const logger = require('../logger')
const response = require('../response')

const imageRouter = module.exports = Router()

// upload image
imageRouter.post('/uploadimage', function (req, res) {
  var form = new formidable.IncomingForm()

  form.keepExtensions = true

  form.parse(req, function (err, fields, files) {
    if (err || !files.image || !files.image.path) {
      response.errorForbidden(req, res)
    } else {
      if (config.debug) {
        logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image))
      }

      const uploadProvider = require('./' + config.imageUploadType)
      uploadProvider.uploadImage(files.image.path, function (err, url) {
        // remove temporary upload file, and ignore any error
        fs.unlink(files.image.path, () => {})
        if (err !== null) {
          logger.error(err)
          return res.status(500).end('upload image error')
        }
        res.send({
          link: url
        })
      })
    }
  })
})

Analysis of the code snippet above reveals that the lower version allows the upload of arbitrary file types, lacking validation mechanisms such as the checkImageValid method in higher versions to verify the legitimacy of uploaded files.

• codimd\lib\csp.js

var config = require('./config')
var uuid = require('uuid')

var CspStrategy = {}

var defaultDirectives = {
  defaultSrc: ['\'self\''],
  scriptSrc: ['\'self\'', 'vimeo.com', 'https://gist.github.com', 'www.slideshare.net', 'https://query.yahooapis.com', '\'unsafe-eval\''],
  // ^ TODO: Remove unsafe-eval - webpack script-loader issues https://github.com/hackmdio/codimd/issues/594
  imgSrc: ['*', 'data:'],
  styleSrc: ['\'self\'', '\'unsafe-inline\'', 'https://github.githubassets.com'], // unsafe-inline is required for some libs, plus used in views
  fontSrc: ['\'self\'', 'data:', 'https://public.slidesharecdn.com'],
  objectSrc: ['*'], // Chrome PDF viewer treats PDFs as objects :/
  mediaSrc: ['*'],
  childSrc: ['*'],
  connectSrc: ['*']
}

var dropboxDirectives = {
  scriptSrc: ['https://www.dropbox.com']
}

var cdnDirectives = {
  scriptSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.jsdelivr.net', 'https://cdn.mathjax.org'],
  styleSrc: ['https://cdnjs.cloudflare.com', 'https://cdn.jsdelivr.net', 'https://fonts.googleapis.com'],
  fontSrc: ['https://cdnjs.cloudflare.com', 'https://fonts.gstatic.com']
}

var disqusDirectives = {
  scriptSrc: ['https://disqus.com', 'https://*.disqus.com', 'https://*.disquscdn.com'],
  styleSrc: ['https://*.disquscdn.com'],
  fontSrc: ['https://*.disquscdn.com']
}

var googleAnalyticsDirectives = {
  scriptSrc: ['https://www.google-analytics.com']
}

CspStrategy.computeDirectives = function () {
  var directives = {}
  mergeDirectives(directives, config.csp.directives)
  mergeDirectivesIf(config.csp.addDefaults, directives, defaultDirectives)
  mergeDirectivesIf(config.useCDN, directives, cdnDirectives)
  mergeDirectivesIf(config.dropbox && config.dropbox.appKey, directives, dropboxDirectives)
  mergeDirectivesIf(config.csp.addDisqus, directives, disqusDirectives)
  mergeDirectivesIf(config.csp.addGoogleAnalytics, directives, googleAnalyticsDirectives)
  if (!areAllInlineScriptsAllowed(directives)) {
    addInlineScriptExceptions(directives)
  }
  addUpgradeUnsafeRequestsOptionTo(directives)
  addReportURI(directives)
  return directives
}

function mergeDirectives (existingDirectives, newDirectives) {
  for (var propertyName in newDirectives) {
    var newDirective = newDirectives[propertyName]
    if (newDirective) {
      var existingDirective = existingDirectives[propertyName] || []
      existingDirectives[propertyName] = existingDirective.concat(newDirective)
    }
  }
}

function mergeDirectivesIf (condition, existingDirectives, newDirectives) {
  if (condition) {
    mergeDirectives(existingDirectives, newDirectives)
  }
}

function areAllInlineScriptsAllowed (directives) {
  return directives.scriptSrc.indexOf('\'unsafe-inline\'') !== -1
}

function addInlineScriptExceptions (directives) {
  directives.scriptSrc.push(getCspNonce)
  // TODO: This is the SHA-256 hash of the inline script in build/reveal.js/plugins/notes/notes.html
  // Any more clean solution appreciated.
  directives.scriptSrc.push('\'sha256-81acLZNZISnyGYZrSuoYhpzwDTTxi7vC1YM4uNxqWaM=\'')
}

function getCspNonce (req, res) {
  return "'nonce-" + res.locals.nonce + "'"
}

function addUpgradeUnsafeRequestsOptionTo (directives) {
  if (config.csp.upgradeInsecureRequests === 'auto' && config.useSSL) {
    directives.upgradeInsecureRequests = true
  } else if (config.csp.upgradeInsecureRequests === true) {
    directives.upgradeInsecureRequests = true
  }
}

function addReportURI (directives) {
  if (config.csp.reportURI) {
    directives.reportUri = config.csp.reportURI
  }
}

CspStrategy.addNonceToLocals = function (req, res, next) {
  res.locals.nonce = uuid.v4()
  next()
}

module.exports = CspStrategy

Examination of this code snippet indicates that the CSP policy allows the loading of JavaScript scripts from same-origin and whitelisted domains. However, for inline JavaScript code, it generates a random nonce value using UUID and maintains a whitelist of specific SHA-256 hash values to ensure that only particular inline scripts can be executed. Consequently, we cannot directly employ inline JavaScript methods to trigger XSS vectors.

We attempted to access the URL returned in the HTTP response in the browser, but found that the XSS vector did not execute successfully.

---

Test Procedure & Proof Of Concept

• Authentication is not necessary.

However, through meticulous analysis, you may also discover that there are two scenarios in which this CSP policy restriction can be circumvented.

---

Exploitation Scenario - One

If files are uploaded to a same-origin server, attackers can initially upload a JavaScript file containing malicious code, followed by uploading an HTML file that references this JavaScript file, thereby triggering the XSS vector.

When accessing the URL http://127.0.0.1:3000/uploads/upload_c5af4d0dc7f824abb474e10e5842dcfb.html returned in the HTTP response via a browser, the XSS vector was successfully executed, thereby circumventing the CSP policy.

---

Exploitation Scenario - Two

Another exploitation scenario is also evident: since the CSP policy is set in the server-side response headers of the current web application, it only applies to content under the current website's same-origin domain. If files are uploaded to a remote server, such as an S3 server, the CSP policy will not be effective.

---

Impact

While the developers implemented multiple security controls including allowlist mechanisms and Content Security Policy (CSP) protections to mitigate potential threats, the diversity and complexity of deployment environments combined with implementation discrepancies create sufficient attack surface for sophisticated threat actors to circumvent these safeguards.

Notable implementation differences exist between higher versions (v2.5.4 - v2.2.1) and lower versions (≤v2.2.0), resulting in distinct security defects and exploitation vectors, under specific conditions, allows attackers to upload malicious files, potentially creating attack vectors for stored Cross-Site Scripting (XSS).

[jackycute]

PR welcome!

[PelaezLink]

Hello all,

We detected a related issue with the /uploadimage endpoint. We are still using 2.5.4 but based on the release notes I think this will still be an issue in 2.6.0. Basically, the problem is that CodiMD allows unauthenticated users to upload files to the endpoint. In our case we use SSO to protect some of the notes, others are public to view, but to edit or create notes you always need to be logged in through SSO.

This is not a problem when you are pasting an image on a note, because to be able to edit view/note you need to logged in, so this means that unauthenticated users cannot access that flow. Nevertheless, it is possible for external users to upload files through the endpoint without any authentication, since they don not have to log in the Web UI. With a simple curl:

curl -iF image=@example_image.jpeg https://codimd-instance.web.com/uploadimage
HTTP/1.1 200 OK

The fix we implemented is just checking in the function to upload images that the user is logged in:

• lib/imageRouter/index.js

'use strict'

const fs = require('fs')
const path = require('path')
const Router = require('express').Router
const formidable = require('formidable')

const readChunk = require('read-chunk')
const imageType = require('image-type')
const mime = require('mime-types')

const config = require('../config')
const logger = require('../logger')
const response = require('../response')

const imageRouter = module.exports = Router()

function checkImageValid (filepath) {
  try {
    const buffer = readChunk.sync(filepath, 0, 12)
    /** @type {{ ext: string, mime: string } | null} */
    const mimetypeFromBuf = imageType(buffer)
    const mimeTypeFromExt = mime.lookup(path.extname(filepath))

    return mimetypeFromBuf && config.allowedUploadMimeTypes.includes(mimetypeFromBuf.mime) &&
          mimeTypeFromExt && config.allowedUploadMimeTypes.includes(mimeTypeFromExt)
  } catch (err) {
    logger.error(err)
    return false
  }
}

// upload image
imageRouter.post('/uploadimage', function (req, res) {
  const isLogin = req.isAuthenticated()

  if (!isLogin) {
    return res.status(403).send('Unauthenticated')
  }

  var form = new formidable.IncomingForm({
    keepExtensions: true
  })

  form.parse(req, function (err, fields, files) {
    if (err || !files.image || !files.image.filepath) {
      response.errorForbidden(req, res)
    } else {
      if (config.debug) {
        logger.info('SERVER received uploadimage: ' + JSON.stringify(files.image))
      }

      if (!checkImageValid(files.image.filepath)) {
        return response.errorForbidden(req, res)
      }

      const uploadProvider = require('./' + config.imageUploadType)
      uploadProvider.uploadImage(files.image.filepath, function (err, url) {
        // remove temporary upload file, and ignore any error
        fs.unlink(files.image.filepath, () => {})
        if (err !== null) {
          logger.error(err)
          return res.status(500).end('upload image error')
        }
        res.send({
          link: url
        })
      })
    }
  })
})

As you can see, the part we added is:

  const isLogin = req.isAuthenticated()

  if (!isLogin) {
    return res.status(403).send('Unauthenticated')
  }

This way only authenticated users can upload files:

curl -iF image=@example_image.jpeg https://codimd-instance.web.com/uploadimage
HTTP/1.1 403 Forbidden

I am curious if someone else had a similar issue and if you consider this is a good fix for the general use case I can open PR. Happy to hear suggestions. Thanks a lot.