فهرست منبع

Merge pull request #2083 from transloadit/validate-url

companion: add stronger validation for urls sent via URL plugin
Ifedapo .A. Olarewaju 5 سال پیش
والد
کامیت
d98384989f

+ 42 - 0
package-lock.json

@@ -8067,6 +8067,7 @@
         "express-session": "1.15.6",
         "grant": "4.6.5",
         "helmet": "3.21.2",
+        "ip-address": "6.2.0",
         "isobject": "3.0.1",
         "jsonwebtoken": "8.5.1",
         "lodash.merge": "4.6.2",
@@ -22500,6 +22501,32 @@
       "resolved": "https://registry.npmjs.org/ip/-/ip-1.1.5.tgz",
       "integrity": "sha1-vd7XARQpCCjAoDnnLvJfWq7ENUo="
     },
+    "ip-address": {
+      "version": "6.2.0",
+      "resolved": "https://registry.npmjs.org/ip-address/-/ip-address-6.2.0.tgz",
+      "integrity": "sha512-7G/8LVMRqM11pLcXx3PlX9rlqenMVUbppAc2sMvz+Ef0mUFm++cecpcEwb+Wfcdt2apu5XLTm9ox+Xz/TB7TGg==",
+      "requires": {
+        "jsbn": "1.1.0",
+        "lodash.find": "4.6.0",
+        "lodash.max": "4.0.1",
+        "lodash.merge": "4.6.2",
+        "lodash.padstart": "4.6.1",
+        "lodash.repeat": "4.1.0",
+        "sprintf-js": "1.1.2"
+      },
+      "dependencies": {
+        "jsbn": {
+          "version": "1.1.0",
+          "resolved": "https://registry.npmjs.org/jsbn/-/jsbn-1.1.0.tgz",
+          "integrity": "sha1-sBMHyym2GKHtJux56RH4A8TaAEA="
+        },
+        "sprintf-js": {
+          "version": "1.1.2",
+          "resolved": "https://registry.npmjs.org/sprintf-js/-/sprintf-js-1.1.2.tgz",
+          "integrity": "sha512-VE0SOVEHCk7Qc8ulkWw3ntAzXuqf7S2lvwQaDLRnUeIEaKNQJzV6BwmLKhOqT61aGhfUMrXeaBk+oDGCzvhcug=="
+        }
+      }
+    },
     "ip-regex": {
       "version": "2.1.0",
       "resolved": "https://registry.npmjs.org/ip-regex/-/ip-regex-2.1.0.tgz",
@@ -25654,6 +25681,11 @@
       "resolved": "https://registry.npmjs.org/lodash.filter/-/lodash.filter-4.6.0.tgz",
       "integrity": "sha1-ZosdSYFgOuHMWm+nYBQ+SAtMSs4="
     },
+    "lodash.find": {
+      "version": "4.6.0",
+      "resolved": "https://registry.npmjs.org/lodash.find/-/lodash.find-4.6.0.tgz",
+      "integrity": "sha1-ywcE1Hq3F4n/oN6Ll92Sb7iLE7E="
+    },
     "lodash.flatten": {
       "version": "4.4.0",
       "resolved": "https://registry.npmjs.org/lodash.flatten/-/lodash.flatten-4.4.0.tgz",
@@ -25739,6 +25771,11 @@
       "resolved": "https://registry.npmjs.org/lodash.mapvalues/-/lodash.mapvalues-4.6.0.tgz",
       "integrity": "sha1-G6+lAF3p3W9PJmaMMMo3IwzJaJw="
     },
+    "lodash.max": {
+      "version": "4.0.1",
+      "resolved": "https://registry.npmjs.org/lodash.max/-/lodash.max-4.0.1.tgz",
+      "integrity": "sha1-hzVWbGGLNan3YFILSHrnllivE2o="
+    },
     "lodash.memoize": {
       "version": "3.0.4",
       "resolved": "https://registry.npmjs.org/lodash.memoize/-/lodash.memoize-3.0.4.tgz",
@@ -25795,6 +25832,11 @@
       "resolved": "https://registry.npmjs.org/lodash.reject/-/lodash.reject-4.6.0.tgz",
       "integrity": "sha1-gNZJLcFHCGS79YNTO2UfQqn1JBU="
     },
+    "lodash.repeat": {
+      "version": "4.1.0",
+      "resolved": "https://registry.npmjs.org/lodash.repeat/-/lodash.repeat-4.1.0.tgz",
+      "integrity": "sha1-/H3oEx2MisB+S0n3T/6CnR8r7EQ="
+    },
     "lodash.set": {
       "version": "4.3.2",
       "resolved": "https://registry.npmjs.org/lodash.set/-/lodash.set-4.3.2.tgz",

+ 1 - 0
packages/@uppy/companion/package.json

@@ -45,6 +45,7 @@
     "express-session": "1.15.6",
     "grant": "4.6.5",
     "helmet": "3.21.2",
+    "ip-address": "6.2.0",
     "isobject": "3.0.1",
     "jsonwebtoken": "8.5.1",
     "lodash.merge": "4.6.2",

+ 33 - 6
packages/@uppy/companion/src/server/controllers/url.js

@@ -3,6 +3,7 @@ const request = require('request')
 const Uploader = require('../Uploader')
 const validator = require('validator')
 const utils = require('../helpers/utils')
+const { getProtectedHttpAgent } = require('../helpers/request')
 const logger = require('../logger')
 
 module.exports = () => {
@@ -19,13 +20,13 @@ module.exports = () => {
  */
 const meta = (req, res) => {
   logger.debug('URL file import handler running', null, req.id)
-
-  if (!validator.isURL(req.body.url, { require_protocol: true, require_tld: !req.companion.options.debug })) {
+  const debug = req.companion.options.debug
+  if (!validateURL(req.body.url, debug)) {
     logger.debug('Invalid request body detected. Exiting url meta handler.', null, req.id)
     return res.status(400).json({ error: 'Invalid request body' })
   }
 
-  utils.getURLMeta(req.body.url)
+  utils.getURLMeta(req.body.url, !debug)
     .then((meta) => res.json(meta))
     .catch((err) => {
       logger.error(err, 'controller.url.meta.error', req.id)
@@ -42,6 +43,11 @@ const meta = (req, res) => {
  */
 const get = (req, res) => {
   logger.debug('URL file import handler running', null, req.id)
+  const debug = req.companion.options.debug
+  if (!validateURL(req.body.url, debug)) {
+    logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id)
+    return res.status(400).json({ error: 'Invalid request body' })
+  }
 
   utils.getURLMeta(req.body.url)
     .then(({ size }) => {
@@ -58,30 +64,51 @@ const get = (req, res) => {
       logger.debug('Waiting for socket connection before beginning remote download.', null, req.id)
       uploader.onSocketReady(() => {
         logger.debug('Socket connection received. Starting remote download.', null, req.id)
-        downloadURL(req.body.url, uploader.handleChunk.bind(uploader), req.id)
+        downloadURL(req.body.url, uploader.handleChunk.bind(uploader), !debug, req.id)
       })
 
       const response = uploader.getResponse()
       res.status(response.status).json(response.body)
     }).catch((err) => {
       logger.error(err, 'controller.url.get.error', req.id)
+      // @todo this should send back error (not err)
       res.json({ err })
     })
 }
 
+/**
+ * Validates that the download URL is secure
+ * @param {string} url the url to validate
+ * @param {boolean} debug whether the server is running in debug mode
+ */
+const validateURL = (url, debug) => {
+  const validURLOpts = {
+    protocols: ['http', 'https'],
+    require_protocol: true,
+    require_tld: !debug
+  }
+  if (!validator.isURL(url, validURLOpts)) {
+    return false
+  }
+
+  return true
+}
+
 /**
  * Downloads the content in the specified url, and passes the data
  * to the callback chunk by chunk.
  *
  * @param {string} url
  * @param {typeof Function} onDataChunk
+ * @param {boolean} blockLocalIPs
  * @param {string=} traceId
  */
-const downloadURL = (url, onDataChunk, traceId) => {
+const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => {
   const opts = {
     uri: url,
     method: 'GET',
-    followAllRedirects: true
+    followAllRedirects: true,
+    agentClass: getProtectedHttpAgent(utils.parseURL(url).protocol, blockLocalIPs)
   }
 
   request(opts)

+ 132 - 0
packages/@uppy/companion/src/server/helpers/request.js

@@ -0,0 +1,132 @@
+const http = require('http')
+const https = require('https')
+const dns = require('dns')
+const ipAddress = require('ip-address')
+const FORBIDDEN_IP_ADDRESS = 'Forbidden IP address'
+
+function isIPAddress (address) {
+  const addressAsV6 = new ipAddress.Address6(address)
+  const addressAsV4 = new ipAddress.Address4(address)
+  return addressAsV6.isValid() || addressAsV4.isValid()
+}
+
+/**
+ * Determine if a IP address provided is a private one.
+ * Return TRUE if it's the case, FALSE otherwise.
+ * Excerpt from:
+ * https://cheatsheetseries.owasp.org/cheatsheets/Server_Side_Request_Forgery_Prevention_Cheat_Sheet.html#case-2---application-can-send-requests-to-any-external-ip-address-or-domain-name
+ *
+ * @param {string} ipAddress the ip address to validate
+ * @returns {boolean}
+ */
+function isPrivateIP (ipAddress) {
+  let isPrivate = false
+  // Build the list of IP prefix for V4 and V6 addresses
+  const ipPrefix = []
+  // Add prefix for loopback addresses
+  ipPrefix.push('127.')
+  ipPrefix.push('0.')
+  // Add IP V4 prefix for private addresses
+  // See https://en.wikipedia.org/wiki/Private_network
+  ipPrefix.push('10.')
+  ipPrefix.push('172.16.')
+  ipPrefix.push('172.17.')
+  ipPrefix.push('172.18.')
+  ipPrefix.push('172.19.')
+  ipPrefix.push('172.20.')
+  ipPrefix.push('172.21.')
+  ipPrefix.push('172.22.')
+  ipPrefix.push('172.23.')
+  ipPrefix.push('172.24.')
+  ipPrefix.push('172.25.')
+  ipPrefix.push('172.26.')
+  ipPrefix.push('172.27.')
+  ipPrefix.push('172.28.')
+  ipPrefix.push('172.29.')
+  ipPrefix.push('172.30.')
+  ipPrefix.push('172.31.')
+  ipPrefix.push('192.168.')
+  ipPrefix.push('169.254.')
+  // Add IP V6 prefix for private addresses
+  // See https://en.wikipedia.org/wiki/Unique_local_address
+  // See https://en.wikipedia.org/wiki/Private_network
+  // See https://simpledns.com/private-ipv6
+  ipPrefix.push('fc')
+  ipPrefix.push('fd')
+  ipPrefix.push('fe')
+  ipPrefix.push('ff')
+  ipPrefix.push('::1')
+  // Verify the provided IP address
+  // Remove whitespace characters from the beginning/end of the string
+  // and convert it to lower case
+  // Lower case is for preventing any IPV6 case bypass using mixed case
+  // depending on the source used to get the IP address
+  const ipToVerify = ipAddress.trim().toLowerCase()
+  // Perform the check against the list of prefix
+  for (const prefix of ipPrefix) {
+    if (ipToVerify.startsWith(prefix)) {
+      isPrivate = true
+      break
+    }
+  }
+
+  return isPrivate
+}
+
+module.exports.FORBIDDEN_IP_ADDRESS = FORBIDDEN_IP_ADDRESS
+
+/**
+ * Returns http Agent that will prevent requests to private IPs (to preven SSRF)
+ * @param {string} protocol http or https protocol needed for the request
+ * @param {boolean} blockPrivateIPs if set to false, this protection will be disabled
+ */
+module.exports.getProtectedHttpAgent = (protocol, blockPrivateIPs) => {
+  if (blockPrivateIPs) {
+    return protocol === 'https' ? HttpsAgent : HttpAgent
+  }
+
+  return protocol === 'https' ? https.Agent : http.Agent
+}
+
+function dnsLookup (hostname, options, callback) {
+  dns.lookup(hostname, options, (err, addresses, maybeFamily) => {
+    if (err) {
+      callback(err, addresses, maybeFamily)
+      return
+    }
+
+    const toValidate = Array.isArray(addresses) ? addresses : [{ address: addresses }]
+    for (const record of toValidate) {
+      if (isPrivateIP(record.address)) {
+        callback(new Error(FORBIDDEN_IP_ADDRESS), addresses, maybeFamily)
+        return
+      }
+    }
+
+    callback(err, addresses, maybeFamily)
+  })
+}
+
+class HttpAgent extends http.Agent {
+  createConnection (options, callback) {
+    options.lookup = dnsLookup
+    if (isIPAddress(options.host) && isPrivateIP(options.host)) {
+      callback(new Error(FORBIDDEN_IP_ADDRESS))
+      return
+    }
+    // @ts-ignore
+    return super.createConnection(options, callback)
+  }
+}
+
+class HttpsAgent extends https.Agent {
+  createConnection (options, callback) {
+    options.lookup = dnsLookup
+    if (isIPAddress(options.host) && isPrivateIP(options.host)) {
+      callback(new Error(FORBIDDEN_IP_ADDRESS))
+      return
+    }
+    // @ts-ignore
+    return super.createConnection(options, callback)
+  }
+}

+ 17 - 2
packages/@uppy/companion/src/server/helpers/utils.js

@@ -1,5 +1,7 @@
 const request = require('request')
+const urlParser = require('url')
 const crypto = require('crypto')
+const { getProtectedHttpAgent } = require('./request')
 
 /**
  *
@@ -41,18 +43,31 @@ exports.sanitizeHtml = (text) => {
   return text ? text.replace(/<\/?[^>]+(>|$)/g, '') : text
 }
 
+/**
+ * Node 6(and beyond) compatible url parser
+ * @todo drop the use of url.parse when support for node 6 is dropped
+ *
+ * @param {string} url URL to be parsed
+ */
+exports.parseURL = (url) => {
+  // eslint-disable-next-line
+  return urlParser.URL ? new urlParser.URL(url) : urlParser.parse(url)
+}
+
 /**
  * Gets the size and content type of a url's content
  *
  * @param {string} url
+ * @param {boolean=} blockLocalIPs
  * @return {Promise}
  */
-exports.getURLMeta = (url) => {
+exports.getURLMeta = (url, blockLocalIPs = false) => {
   return new Promise((resolve, reject) => {
     const opts = {
       uri: url,
       method: 'HEAD',
-      followAllRedirects: true
+      followAllRedirects: true,
+      agentClass: getProtectedHttpAgent(exports.parseURL(url).protocol, blockLocalIPs)
     }
 
     request(opts, (err, response, body) => {

+ 2 - 5
packages/@uppy/companion/src/standalone/index.js

@@ -1,11 +1,11 @@
 const express = require('express')
 const qs = require('querystring')
-const urlParser = require('url')
 const companion = require('../companion')
 const helmet = require('helmet')
 const morgan = require('morgan')
 const bodyParser = require('body-parser')
 const redis = require('../server/redis')
+const { parseURL } = require('../server/helpers/utils')
 const merge = require('lodash.merge')
 // @ts-ignore
 const promBundle = require('express-prom-bundle')
@@ -53,10 +53,7 @@ morgan.token('url', (req, res) => {
 morgan.token('referrer', (req, res) => {
   const ref = req.headers.referer || req.headers.referrer
   if (typeof ref === 'string') {
-    // @todo drop the use of url.parse
-    // when support for node 6 is dropped
-    // eslint-disable-next-line
-    const parsed = urlParser.URL ? new urlParser.URL(ref) : urlParser.parse(ref)
+    const parsed = parseURL(ref)
     const query = qs.parse(parsed.search.replace('?', ''));
     ['uppyAuthToken', 'access_token'].forEach(key => {
       if (query[key]) {

+ 2 - 2
packages/@uppy/companion/test/__tests__/header-blacklist.js

@@ -5,7 +5,7 @@ const headerSanitize = require('../../src/server/header-blacklist')
 describe('Header black-list testing', () => {
   test('All headers invalid by name', () => {
     const headers = headerSanitize({
-      origin: 'http://www.google.com',
+      origin: 'http://www.transloadit.com',
       'Accept-Charset': '...',
       'content-Length': 1234
     })
@@ -40,7 +40,7 @@ describe('Header black-list testing', () => {
       'Content-Type': 'application/json',
       'Content-Length': 1234,
       Expires: 'Wed, 21 Oct 2015 07:28:00 GMT',
-      Origin: 'http://www.google.com'
+      Origin: 'http://www.transloadit.com'
     })
     expect(Object.keys(headers)).toHaveLength(3)
     expect(headers).toHaveProperty('Authorization')

+ 80 - 0
packages/@uppy/companion/test/__tests__/http-agent.js

@@ -0,0 +1,80 @@
+/* global test:false, expect:false, describe:false, */
+
+const { getProtectedHttpAgent, FORBIDDEN_IP_ADDRESS } = require('../../src/server/helpers/request')
+const request = require('request')
+
+describe('test protected request Agent', () => {
+  test('allows URLs without IP addresses', (done) => {
+    const options = {
+      uri: 'https://www.transloadit.com',
+      method: 'GET',
+      agentClass: getProtectedHttpAgent('https', true)
+    }
+
+    request(options, (err) => {
+      if (err) {
+        expect(err.message).not.toEqual(FORBIDDEN_IP_ADDRESS)
+        expect(err.message.startsWith(FORBIDDEN_IP_ADDRESS)).toEqual(false)
+        done()
+      } else {
+        done()
+      }
+    })
+  })
+
+  test('blocks private http IP address', (done) => {
+    const options = {
+      uri: 'http://172.20.10.4:8090',
+      method: 'GET',
+      agentClass: getProtectedHttpAgent('http', true)
+    }
+
+    request(options, (err) => {
+      expect(err).toBeInstanceOf(Error)
+      expect(err.message).toEqual(FORBIDDEN_IP_ADDRESS)
+      done()
+    })
+  })
+
+  test('blocks private https IP address', (done) => {
+    const options = {
+      uri: 'https://172.20.10.4:8090',
+      method: 'GET',
+      agentClass: getProtectedHttpAgent('https', true)
+    }
+
+    request(options, (err) => {
+      expect(err).toBeInstanceOf(Error)
+      expect(err.message).toEqual(FORBIDDEN_IP_ADDRESS)
+      done()
+    })
+  })
+
+  test('blocks localhost IP address', (done) => {
+    const options = {
+      uri: 'http://127.0.0.1:8090',
+      method: 'GET',
+      agentClass: getProtectedHttpAgent('http', true)
+    }
+
+    request(options, (err) => {
+      expect(err).toBeInstanceOf(Error)
+      expect(err.message).toEqual(FORBIDDEN_IP_ADDRESS)
+      done()
+    })
+  })
+
+  test('blocks URLs that have DNS pinned to a private IP address', (done) => {
+    const options = {
+      uri: 'http://127.0.0.1.xip.io:8090',
+      method: 'GET',
+      agentClass: getProtectedHttpAgent('http', true)
+    }
+
+    request(options, (err) => {
+      expect(err).toBeTruthy()
+      expect(err.message.startsWith(FORBIDDEN_IP_ADDRESS)).toEqual(true)
+      done()
+    })
+  })
+})