CVE-2022-0528: Server-Side Request Forgery (SSRF) (CWE-918)
value: Server-Side Request Forgery (SSRF) in GitHub repository transloadit/uppy prior to 3.3.1.


CWE description: The web server receives a URL or similar request from an upstream component and retrieves the contents of this URL, but it does not sufficiently ensure that the request is being sent to the expected destination.
CVSS v3 base score: 7.5
Published: 2022-03-03T07:15Z
Repository: https://github.com/transloadit/uppy
Language: JavaScript
Files changed: .env.example, bin/companion.sh, packages/@uppy/companion/src/companion.js, packages/@uppy/companion/src/server/controllers/url.js, packages/@uppy/companion/src/server/logger.js, packages/@uppy/companion/src/server/provider/facebook/index.js, packages/@uppy/companion/src/server/provider/instagram/graph/index.js, packages/@uppy/companion/src/server/provider/unsplash/index.js, packages/@uppy/companion/src/standalone/helper.js, packages/@uppy/companion/test/__tests__/companion.js, packages/@uppy/companion/test/mockserver.js, website/src/docs/companion.md
Commit date: 2022-03-02 19:02:56 +0800
Commit message: replace debug

remove debug flag
instead add allowLocalUrls (COMPANION_ALLOW_LOCAL_URLS)
which defaults to false
this fixes https://huntr.dev/bounties/8b060cc3-2420-468e-8293-b9216620175b/

also don't allow localhost for any providers getURLMeta

Diff: diff --git a/.env.example b/.env.example
index a3c3f3769..3fd907211 100644
--- a/.env.example
+++ b/.env.example
@@ -9,10 +9,13 @@ COMPANION_DOMAIN=localhost:3020
 COMPANION_PROTOCOL=http
 COMPANION_PORT=3020
 COMPANION_CLIENT_ORIGINS=
 COMPANION_SECRET=development
 
+# NOTE: Only enable this in development. Enabling it in production is a security risk
+COMPANION_ALLOW_LOCAL_URLS=true
+
 COMPANION_DROPBOX_KEY=***
 COMPANION_DROPBOX_SECRET=***
 
 COMPANION_BOX_KEY=***
 COMPANION_BOX_SECRET=***
diff --git a/bin/companion.sh b/bin/companion.sh
index 43b9437a4..df114b8b3 100755
--- a/bin/companion.sh
+++ b/bin/companion.sh
@@ -9,8 +9,9 @@ else
     COMPANION_DOMAIN="localhost:3020" \
     COMPANION_PROTOCOL="http" \
     COMPANION_PORT=3020 \
     COMPANION_CLIENT_ORIGINS="" \
     COMPANION_SECRET="development" \
+    COMPANION_ALLOW_LOCAL_URLS="true" \
     nodemon --watch packages/@uppy/companion/src --exec node ./packages/@uppy/companion/src/standalone/start-server.js
 fi
 
diff --git a/packages/@uppy/companion/src/companion.js b/packages/@uppy/companion/src/companion.js
index 964b1c200..2135fd431 100644
--- a/packages/@uppy/companion/src/companion.js
+++ b/packages/@uppy/companion/src/companion.js
@@ -39,11 +39,11 @@ const defaultOptions = {
       useAccelerateEndpoint: false,
       getKey: (req, filename) => filename,
       expires: ms('5 minutes') / 1000,
     },
   },
-  debug: true,
+  allowLocalUrls: false,
   logClientVersion: true,
   periodicPingUrls: [],
   streamingUpload: false,
 }
 
diff --git a/packages/@uppy/companion/src/server/controllers/url.js b/packages/@uppy/companion/src/server/controllers/url.js
index b7bc41c5f..088e2f66c 100644
--- a/packages/@uppy/companion/src/server/controllers/url.js
+++ b/packages/@uppy/companion/src/server/controllers/url.js
@@ -9,21 +9,21 @@ const logger = require('../logger')
 
 /**
  * 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
+ * @param {boolean} ignoreTld whether to allow local addresses
  */
-const validateURL = (url, debug) => {
+const validateURL = (url, ignoreTld) => {
   if (!url) {
     return false
   }
 
   const validURLOpts = {
     protocols: ['http', 'https'],
     require_protocol: true,
-    require_tld: !debug,
+    require_tld: !ignoreTld,
   }
   if (!validator.isURL(url, validURLOpts)) {
     return false
   }
 
@@ -81,17 +81,17 @@ const downloadURL = async (url, blockLocalIPs, traceId) => {
  * @param {object} res expressJS response object
  */
 const meta = async (req, res) => {
   try {
     logger.debug('URL file import handler running', null, req.id)
-    const { debug } = req.companion.options
-    if (!validateURL(req.body.url, debug)) {
+    const { allowLocalUrls } = req.companion.options
+    if (!validateURL(req.body.url, allowLocalUrls)) {
       logger.debug('Invalid request body detected. Exiting url meta handler.', null, req.id)
       return res.status(400).json({ error: 'Invalid request body' })
     }
 
-    const urlMeta = await getURLMeta(req.body.url, !debug)
+    const urlMeta = await getURLMeta(req.body.url, !allowLocalUrls)
     return res.json(urlMeta)
   } 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' })
@@ -105,24 +105,24 @@ const meta = async (req, res) => {
  * @param {object} req expressJS request object
  * @param {object} res expressJS response object
  */
 const get = async (req, res) => {
   logger.debug('URL file import handler running', null, req.id)
-  const { debug } = req.companion.options
-  if (!validateURL(req.body.url, debug)) {
+  const { allowLocalUrls } = req.companion.options
+  if (!validateURL(req.body.url, allowLocalUrls)) {
     logger.debug('Invalid request body detected. Exiting url import handler.', null, req.id)
     res.status(400).json({ error: 'Invalid request body' })
     return
   }
 
   async function getSize () {
-    const { size } = await getURLMeta(req.body.url, !debug)
+    const { size } = await getURLMeta(req.body.url, !allowLocalUrls)
     return size
   }
 
   async function download () {
-    return downloadURL(req.body.url, !debug, req.id)
+    return downloadURL(req.body.url, !allowLocalUrls, req.id)
   }
 
   function onUnhandledError (err) {
     logger.error(err, 'controller.url.error', req.id)
     // @todo send more meaningful error message and status code to client if possible
diff --git a/packages/@uppy/companion/src/server/logger.js b/packages/@uppy/companion/src/server/logger.js
index 68d5563e9..7972ba945 100644
--- a/packages/@uppy/companion/src/server/logger.js
+++ b/packages/@uppy/companion/src/server/logger.js
@@ -110,11 +110,10 @@ exports.error = (msg, tag, traceId, shouldLogStackTrace) => {
  * @param {string} msg the message to log
  * @param {string=} tag a unique tag to easily search for this message
  * @param {string=} traceId a unique id to easily trace logs tied to a request
  */
 exports.debug = (msg, tag, traceId) => {
-  // @todo: this function should depend on companion's debug option instead
   if (process.env.NODE_ENV !== 'production') {
     // @ts-ignore
     log(msg, tag, 'debug', traceId, chalk.bold.blue)
   }
 }
diff --git a/packages/@uppy/companion/src/server/provider/facebook/index.js b/packages/@uppy/companion/src/server/provider/facebook/index.js
index 6e684dbdb..826456ffa 100644
--- a/packages/@uppy/companion/src/server/provider/facebook/index.js
+++ b/packages/@uppy/companion/src/server/provider/facebook/index.js
@@ -125,11 +125,11 @@ class Facebook extends Provider {
           err = this._error(err, resp)
           logger.error(err, 'provider.facebook.size.error')
           return done(err)
         }
 
-        getURLMeta(this._getMediaUrl(body))
+        getURLMeta(this._getMediaUrl(body), true)
           .then(({ size }) => done(null, size))
           .catch((err2) => {
             logger.error(err2, 'provider.facebook.size.error')
             done(err2)
           })
diff --git a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js
index 79b380d5c..95ecef61f 100644
--- a/packages/@uppy/companion/src/server/provider/instagram/graph/index.js
+++ b/packages/@uppy/companion/src/server/provider/instagram/graph/index.js
@@ -116,11 +116,11 @@ class Instagram extends Provider {
           err = this._error(err, resp)
           logger.error(err, 'provider.instagram.size.error')
           return done(err)
         }
 
-        getURLMeta(body.media_url)
+        getURLMeta(body.media_url, true)
           .then(({ size }) => done(null, size))
           .catch((err2) => {
             logger.error(err2, 'provider.instagram.size.error')
             done(err2)
           })
diff --git a/packages/@uppy/companion/src/server/provider/unsplash/index.js b/packages/@uppy/companion/src/server/provider/unsplash/index.js
index d3b14648a..64d6be2bf 100644
--- a/packages/@uppy/companion/src/server/provider/unsplash/index.js
+++ b/packages/@uppy/companion/src/server/provider/unsplash/index.js
@@ -127,11 +127,11 @@ class Unsplash extends SearchProvider {
         logger.error(error, 'provider.unsplash.size.error')
         done(error)
         return
       }
 
-      getURLMeta(body.links.download)
+      getURLMeta(body.links.download, true)
         .then(({ size }) => done(null, size))
         .catch((err2) => {
           logger.error(err2, 'provider.unsplash.size.error')
           done(err2)
         })
diff --git a/packages/@uppy/companion/src/standalone/helper.js b/packages/@uppy/companion/src/standalone/helper.js
index 0d9a26331..7d87d14c3 100644
--- a/packages/@uppy/companion/src/standalone/helper.js
+++ b/packages/@uppy/companion/src/standalone/helper.js
@@ -104,11 +104,11 @@ const getConfigFromEnv = () => {
     redisOptions: {},
     sendSelfEndpoint: process.env.COMPANION_SELF_ENDPOINT,
     uploadUrls: uploadUrls ? uploadUrls.split(',') : null,
     secret: getSecret('COMPANION_SECRET') || generateSecret(),
     preAuthSecret: getSecret('COMPANION_PREAUTH_SECRET') || generateSecret(),
-    debug: process.env.NODE_ENV && process.env.NODE_ENV !== 'production',
+    allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === 'true',
     // cookieDomain is kind of a hack to support distributed systems. This should be improved but we never got so far.
     cookieDomain: process.env.COMPANION_COOKIE_DOMAIN,
     multipleInstances: true,
     streamingUpload: process.env.COMPANION_STREAMING_UPLOAD === 'true',
     maxFileSize: process.env.COMPANION_MAX_FILE_SIZE ? parseInt(process.env.COMPANION_MAX_FILE_SIZE, 10) : undefined,
diff --git a/packages/@uppy/companion/test/__tests__/companion.js b/packages/@uppy/companion/test/__tests__/companion.js
index 1e011b78b..48b0115f7 100644
--- a/packages/@uppy/companion/test/__tests__/companion.js
+++ b/packages/@uppy/companion/test/__tests__/companion.js
@@ -169,9 +169,37 @@ it('periodically pings', (done) => {
     COMPANION_PERIODIC_PING_INTERVAL: '10',
     COMPANION_PERIODIC_PING_COUNT: '1',
   })
 }, 1000)
 
+it('respects allowLocalUrls', async () => {
+  const server = getServer()
+  const url = 'http://localhost/'
+
+  let res
+
+  res = await request(server)
+    .post('/url/meta')
+    .send({ url })
+    .expect(400)
+
+  expect(res.body).toEqual({ error: 'Invalid request body' })
+
+  res = await request(server)
+    .post('/url/get')
+    .send({
+      fileId: url,
+      metadata: {},
+      endpoint: 'http://url.myendpoint.com/files',
+      protocol: 'tus',
+      size: null,
+      url,
+    })
+    .expect(400)
+
+  expect(res.body).toEqual({ error: 'Invalid request body' })
+}, 1000)
+
 afterAll(() => {
   nock.cleanAll()
   nock.restore()
 })
diff --git a/packages/@uppy/companion/test/mockserver.js b/packages/@uppy/companion/test/mockserver.js
index 830b39195..159a2caa1 100644
--- a/packages/@uppy/companion/test/mockserver.js
+++ b/packages/@uppy/companion/test/mockserver.js
@@ -9,10 +9,11 @@ const defaultEnv = {
   COMPANION_SELF_ENDPOINT: 'localhost:3020',
   COMPANION_HIDE_METRICS: 'false',
   COMPANION_HIDE_WELCOME: 'false',
 
   COMPANION_STREAMING_UPLOAD: 'true',
+  COMPANION_ALLOW_LOCAL_URLS : 'false',
 
   COMPANION_PROTOCOL: 'http',
   COMPANION_DATADIR: './test/output',
   COMPANION_SECRET: 'secret',
 
diff --git a/website/src/docs/companion.md b/website/src/docs/companion.md
index 7990dba4f..5aaa6e381 100644
--- a/website/src/docs/companion.md
+++ b/website/src/docs/companion.md
@@ -244,10 +244,13 @@ export COMPANION_SELF_ENDPOINT="THIS SHOULD BE SAME AS YOUR DOMAIN + PATH"
 export COMPANION_UPLOAD_URLS="http://tusd.tusdemo.net/files/,https://tusd.tusdemo.net/files/"
 
 # corresponds to the streamingUpload option
 export COMPANION_STREAMING_UPLOAD=true
 
+# corresponds to the allowLocalUrls option
+export COMPANION_ALLOW_LOCAL_URLS=false
+
 # corresponds to the maxFileSize option
 export COMPANION_MAX_FILE_SIZE="100000000"
 
 # corresponds to the chunkSize option
 export COMPANION_CHUNK_SIZE="50000000"
@@ -307,10 +310,11 @@ const options = {
   secret: 'mysecret',
   uploadUrls: ['https://myuploadurl.com', /^http:\/\/myuploadurl2.com\//],
   debug: true,
   metrics: false,
   streamingUpload: true,
+  allowLocalUrls: false,
   maxFileSize: 100000000,
   periodicPingUrls: [],
   periodicPingInterval: 60000,
   periodicPingStaticPayload: { static: 'payload' },
 }
@@ -357,10 +361,12 @@ const options = {
 
 17. **periodicPingInterval(optional)** - Interval for periodic ping requests (in ms).
 
 18. **periodicPingStaticPayload(optional)** - A `JSON.stringify`-able JavaScript Object that will be sent as part of the JSON body in the period ping requests.
 
+19. **allowLocalUrls(optional)** - A boolean flag to tell Companion whether to allow requesting local URLs. Note: Only enable this in development. **Enabling it in production is a security risk.**
+
 ### Provider Redirect URIs
 
 When generating your provider API keys on their corresponding developer platforms (e.g [Google Developer Console](https://console.developers.google.com/)), you’d need to provide a `redirect URI` for the OAuth authorization process. In general the redirect URI for each provider takes the format:
 
 `http(s)://$YOUR_COMPANION_HOST_NAME/$PROVIDER_NAME/redirect`

Vulnerable code:   debug: true,
 * @param {boolean} debug whether the server is running in debug mode
const validateURL = (url, debug) => {
    require_tld: !debug,
    const { debug } = req.companion.options
    if (!validateURL(req.body.url, debug)) {
    const urlMeta = await getURLMeta(req.body.url, !debug)
  const { debug } = req.companion.options
  if (!validateURL(req.body.url, debug)) {
    const { size } = await getURLMeta(req.body.url, !debug)
    return downloadURL(req.body.url, !debug, req.id)
  // @todo: this function should depend on companion's debug option instead
        getURLMeta(this._getMediaUrl(body))
        getURLMeta(body.media_url)
      getURLMeta(body.links.download)
    debug: process.env.NODE_ENV && process.env.NODE_ENV !== 'production',
Fixed code: # NOTE: Only enable this in development. Enabling it in production is a security risk
COMPANION_ALLOW_LOCAL_URLS=true

    COMPANION_ALLOW_LOCAL_URLS="true" \
  allowLocalUrls: false,
 * @param {boolean} ignoreTld whether to allow local addresses
const validateURL = (url, ignoreTld) => {
    require_tld: !ignoreTld,
    const { allowLocalUrls } = req.companion.options
    if (!validateURL(req.body.url, allowLocalUrls)) {
    const urlMeta = await getURLMeta(req.body.url, !allowLocalUrls)
  const { allowLocalUrls } = req.companion.options
  if (!validateURL(req.body.url, allowLocalUrls)) {
    const { size } = await getURLMeta(req.body.url, !allowLocalUrls)
    return downloadURL(req.body.url, !allowLocalUrls, req.id)
        getURLMeta(this._getMediaUrl(body), true)
        getURLMeta(body.media_url, true)
      getURLMeta(body.links.download, true)
    allowLocalUrls: process.env.COMPANION_ALLOW_LOCAL_URLS === 'true',
it('respects allowLocalUrls', async () => {
  const server = getServer()
  const url = 'http://localhost/'

  let res

  res = await request(server)
    .post('/url/meta')
    .send({ url })
    .expect(400)

  expect(res.body).toEqual({ error: 'Invalid request body' })

  res = await request(server)
    .post('/url/get')
    .send({
      fileId: url,
      metadata: {},
      endpoint: 'http://url.myendpoint.com/files',
      protocol: 'tus',
      size: null,
      url,
    })
    .expect(400)

  expect(res.body).toEqual({ error: 'Invalid request body' })
}, 1000)

  COMPANION_ALLOW_LOCAL_URLS : 'false',
# corresponds to the allowLocalUrls option
export COMPANION_ALLOW_LOCAL_URLS=false

  allowLocalUrls: false,
19. **allowLocalUrls(optional)** - A boolean flag to tell Companion whether to allow requesting local URLs. Note: Only enable this in development. **Enabling it in production is a security risk.**