companion: block redirects to urls with different protocol (#2322)

packages/@uppy/companion/src/server/controllers/url.js

@@ -4,7 +4,7 @@ const { URL } = require('url')
 const Uploader = require('../Uploader')
 const validator = require('validator')
 const utils = require('../helpers/utils')
-const { getProtectedHttpAgent } = require('../helpers/request')
+const { getProtectedHttpAgent, getRedirectEvaluator } = require('../helpers/request')
 const logger = require('../logger')
 
 module.exports = () => {
@@ -31,6 +31,7 @@ const meta = (req, res) => {
     .then((meta) => res.json(meta))
     .catch((err) => {
       logger.error(err, 'controller.url.meta.error', req.id)
+      // @todo send more meaningful error message and status code to client if possible
       return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
     })
 }
@@ -72,6 +73,7 @@ const get = (req, res) => {
       res.status(response.status).json(response.body)
     }).catch((err) => {
       logger.error(err, 'controller.url.get.error', req.id)
+      // @todo send more meaningful error message and status code to client if possible
       return res.status(err.status || 500).json({ message: 'failed to fetch URL metadata' })
     })
 }
@@ -113,12 +115,22 @@ const downloadURL = (url, onDataChunk, blockLocalIPs, traceId) => {
   const opts = {
     uri: url,
     method: 'GET',
-    followAllRedirects: true,
+    followRedirect: getRedirectEvaluator(url, blockLocalIPs),
     agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs)
   }
 
   request(opts)
-    .on('data', (chunk) => onDataChunk(null, chunk))
+    .on('response', (resp) => {
+      if (resp.statusCode >= 300) {
+        const err = new Error(`URL server responded with status: ${resp.statusCode}`)
+        onDataChunk(err, null)
+      } else {
+        resp.on('data', (chunk) => onDataChunk(null, chunk))
+      }
+    })
     .on('end', () => onDataChunk(null, null))
-    .on('error', (err) => logger.error(err, 'controller.url.download.error', traceId))
+    .on('error', (err) => {
+      logger.error(err, 'controller.url.download.error', traceId)
+      onDataChunk(err, null)
+    })
 }

packages/@uppy/companion/src/server/helpers/request.js

@@ -75,6 +75,22 @@ function isPrivateIP (ipAddress) {
 
 module.exports.FORBIDDEN_IP_ADDRESS = FORBIDDEN_IP_ADDRESS
 
+module.exports.getRedirectEvaluator = (requestURL, blockPrivateIPs) => {
+  const protocol = (new URL(requestURL)).protocol
+  return (res) => {
+    if (!blockPrivateIPs) {
+      return true
+    }
+
+    const redirectURL = res.headers.location
+    if (!redirectURL) {
+      return false
+    }
+
+    return new URL(redirectURL).protocol === protocol
+  }
+}
+
 /**
  * Returns http Agent that will prevent requests to private IPs (to preven SSRF)
  * @param {string} protocol http or http: or https: or https protocol needed for the request

packages/@uppy/companion/src/server/helpers/utils.js

@@ -1,7 +1,7 @@
 const request = require('request')
 const { URL } = require('url')
 const crypto = require('crypto')
-const { getProtectedHttpAgent } = require('./request')
+const { getProtectedHttpAgent, getRedirectEvaluator } = require('./request')
 
 /**
  *
@@ -55,12 +55,15 @@ exports.getURLMeta = (url, blockLocalIPs = false) => {
     const opts = {
       uri: url,
       method: 'HEAD',
-      followAllRedirects: true,
+      followRedirect: getRedirectEvaluator(url, blockLocalIPs),
       agentClass: getProtectedHttpAgent((new URL(url)).protocol, blockLocalIPs)
     }
 
-    request(opts, (err, response, body) => {
-      if (err) {
+    request(opts, (err, response) => {
+      if (err || response.statusCode >= 300) {
+        // @todo possibly set a status code in the error object to get a more helpful
+        // hint at what the cause of error is.
+        err = err || new Error(`URL server responded with status: ${response.statusCode}`)
         reject(err)
       } else {
         resolve({

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

@@ -1,10 +1,40 @@
 /* global test:false, expect:false, describe:false, */
 
-const { getProtectedHttpAgent, FORBIDDEN_IP_ADDRESS } = require('../../src/server/helpers/request')
+const { getProtectedHttpAgent, getRedirectEvaluator, FORBIDDEN_IP_ADDRESS } = require('../../src/server/helpers/request')
 const request = require('request')
 const http = require('http')
 const https = require('https')
 
+describe('test getRedirectEvaluator', () => {
+  const httpURL = 'http://uppy.io'
+  const httpsURL = 'https://uppy.io'
+  const httpRedirectResp = {
+    headers: {
+      location: 'http://transloadit.com'
+    }
+  }
+
+  const httpsRedirectResp = {
+    headers: {
+      location: 'https://transloadit.com'
+    }
+  }
+
+  test('when original URL has "https:" as protocol', (done) => {
+    const shouldRedirectHttps = getRedirectEvaluator(httpsURL, true)
+    expect(shouldRedirectHttps(httpsRedirectResp)).toEqual(true)
+    expect(shouldRedirectHttps(httpRedirectResp)).toEqual(false)
+    done()
+  })
+
+  test('when original URL has "http:" as protocol', (done) => {
+    const shouldRedirectHttp = getRedirectEvaluator(httpURL, true)
+    expect(shouldRedirectHttp(httpRedirectResp)).toEqual(true)
+    expect(shouldRedirectHttp(httpsRedirectResp)).toEqual(false)
+    done()
+  })
+})
+
 describe('test getProtectedHttpAgent', () => {
   test('setting "https:" as protocol', (done) => {
     const Agent = getProtectedHttpAgent('https:')