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

.env.example

@@ -11,6 +11,9 @@ 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=***
 

bin/companion.sh

@@ -11,6 +11,7 @@ else
     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
 

packages/@uppy/companion/src/companion.js

@@ -41,7 +41,7 @@ const defaultOptions = {
       expires: ms('5 minutes') / 1000,
     },
   },
-  debug: true,
+  allowLocalUrls: false,
   logClientVersion: true,
   periodicPingUrls: [],
   streamingUpload: false,

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

@@ -11,17 +11,17 @@ 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
@@ -83,13 +83,13 @@ const downloadURL = async (url, blockLocalIPs, traceId) => {
 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)
@@ -107,20 +107,20 @@ const meta = async (req, res) => {
  */
 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) {

packages/@uppy/companion/src/server/logger.js

@@ -112,7 +112,6 @@ exports.error = (msg, tag, traceId, shouldLogStackTrace) => {
  * @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)

packages/@uppy/companion/src/server/provider/facebook/index.js

@@ -127,7 +127,7 @@ class Facebook extends Provider {
           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')

packages/@uppy/companion/src/server/provider/instagram/graph/index.js

@@ -118,7 +118,7 @@ class Instagram extends Provider {
           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')

packages/@uppy/companion/src/server/provider/unsplash/index.js

@@ -129,7 +129,7 @@ class Unsplash extends SearchProvider {
         return
       }
 
-      getURLMeta(body.links.download)
+      getURLMeta(body.links.download, true)
         .then(({ size }) => done(null, size))
         .catch((err2) => {
           logger.error(err2, 'provider.unsplash.size.error')

packages/@uppy/companion/src/standalone/helper.js

@@ -106,7 +106,7 @@ const getConfigFromEnv = () => {
     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,

packages/@uppy/companion/test/__tests__/companion.js

@@ -171,6 +171,34 @@ it('periodically pings', (done) => {
   })
 }, 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()

packages/@uppy/companion/test/mockserver.js

@@ -11,6 +11,7 @@ const defaultEnv = {
   COMPANION_HIDE_WELCOME: 'false',
 
   COMPANION_STREAMING_UPLOAD: 'true',
+  COMPANION_ALLOW_LOCAL_URLS : 'false',
 
   COMPANION_PROTOCOL: 'http',
   COMPANION_DATADIR: './test/output',

website/src/docs/companion.md

@@ -246,6 +246,9 @@ export COMPANION_UPLOAD_URLS="http://tusd.tusdemo.net/files/,https://tusd.tusdem
 # 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"
 
@@ -309,6 +312,7 @@ const options = {
   debug: true,
   metrics: false,
   streamingUpload: true,
+  allowLocalUrls: false,
   maxFileSize: 100000000,
   periodicPingUrls: [],
   periodicPingInterval: 60000,
@@ -359,6 +363,8 @@ const options = {
 
 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: