fix: sanitize source url to avoid attack

.gitignore

@@ -63,3 +63,5 @@ typings/
 # lockfiles
 yarn.lock
 package-lock.json
+
+.vscode

README.md

@@ -14,7 +14,7 @@ npm i fastify-reply-from
 
 ## Compatibility with fastify-multipart
 `fastify-reply-from` and [`fastify-multipart`](https://github.com/fastify/fastify-multipart) should not be registered as sibling plugins nor shold be registered in plugins which have a parent-child relationship.<br> The two plugins are incompatible, in the sense that the behavior of `fastify-reply-from` might not be the expected one when the above-mentioned conditions are not respected.<br> This is due to the fact that `fastify-multipart` consumes the multipart content by parsing it, hence this content is not forwarded to the target service by `fastify-reply-from`.<br>
-However, the two plugins may be used within the same fastify instance, at the condition that they belong to disjoint branches of the fastify plugins hierarchy tree. 
+However, the two plugins may be used within the same fastify instance, at the condition that they belong to disjoint branches of the fastify plugins hierarchy tree.
 
 ## Usage
 
@@ -104,10 +104,10 @@ You can also pass a custom http agents. If you pass the agents, then the http.ag
 proxy.register(require('fastify-reply-from'), {
   base: 'http://localhost:3001/',
   http: {
-    agents: { 
+    agents: {
       'http:': new http.Agent({ keepAliveMsecs: 10 * 60 * 1000 }), // pass in any options from https://nodejs.org/api/http.html#http_new_agent_options
       'https:': new https.Agent({ keepAliveMsecs: 10 * 60 * 1000 })
-               
+
     },
     requestOptions: { // pass in any options from https://nodejs.org/api/http.html#http_http_request_options_callback
       timeout: 5000 // timeout in msecs, defaults to 10000 (10 seconds)
@@ -193,6 +193,8 @@ instance with a `from` method, which will reply to the original request
 __from the desired source__. The options allows to override any part of
 the request or response being sent or received to/from the source.
 
+**Note: If `base` is specified in plugin options, the `source` here should not override the host/origin.**
+
 #### `onResponse(request, reply, res)`
 
 Called when an http response is received from the source.

index.js

@@ -1,7 +1,6 @@
 'use strict'
 
 const fp = require('fastify-plugin')
-const URL = require('url').URL
 const lru = require('tiny-lru')
 const querystring = require('querystring')
 const Stream = require('stream')
@@ -10,7 +9,8 @@ const buildRequest = require('./lib/request')
 const {
   filterPseudoHeaders,
   copyHeaders,
-  stripHttp1ConnectionHeaders
+  stripHttp1ConnectionHeaders,
+  buildURL
 } = require('./lib/utils')
 
 const { TimeoutError } = buildRequest
@@ -42,7 +42,7 @@ module.exports = fp(function from (fastify, opts, next) {
     }
 
     // we leverage caching to avoid parsing the destination URL
-    const url = cache.get(source) || new URL(source, base)
+    const url = cache.get(source) || buildURL(source, base)
     cache.set(source, url)
 
     const sourceHttp2 = req.httpVersionMajor === 2

lib/utils.js

@@ -54,8 +54,21 @@ function stripHttp1ConnectionHeaders (headers) {
   return dest
 }
 
+// issue ref: https://github.com/fastify/fast-proxy/issues/42
+function buildURL (source, reqBase) {
+  const dest = new URL(source, reqBase)
+
+  // if base is specified, source url should not override it
+  if (reqBase && !reqBase.startsWith(dest.origin)) {
+    throw new Error('source must be a relative path string')
+  }
+
+  return dest
+}
+
 module.exports = {
   copyHeaders,
   stripHttp1ConnectionHeaders,
-  filterPseudoHeaders
+  filterPseudoHeaders,
+  buildURL
 }

test/build-url.js

@@ -0,0 +1,42 @@
+'use strict'
+
+const { test } = require('tap')
+const { buildURL } = require('../lib/utils')
+
+test('should produce valid URL', (t) => {
+  t.plan(1)
+  const url = buildURL('/hi', 'http://localhost')
+  t.equal(url.href, 'http://localhost/hi')
+})
+
+test('should produce valid URL', (t) => {
+  t.plan(1)
+  const url = buildURL('http://localhost/hi', 'http://localhost')
+  t.equal(url.href, 'http://localhost/hi')
+})
+
+test('should return same source when base is not specified', (t) => {
+  t.plan(1)
+  const url = buildURL('http://localhost/hi')
+  t.equal(url.href, 'http://localhost/hi')
+})
+
+const errorInputs = [
+  { source: '//10.0.0.10/hi', base: 'http://localhost' },
+  { source: 'http://10.0.0.10/hi', base: 'http://localhost' },
+  { source: 'https://10.0.0.10/hi', base: 'http://localhost' },
+  { source: 'blah://10.0.0.10/hi', base: 'http://localhost' },
+  { source: '//httpbin.org/hi', base: 'http://localhost' },
+  { source: 'urn:foo:bar', base: 'http://localhost' }
+]
+
+test('should throw when trying to override base', (t) => {
+  t.plan(errorInputs.length)
+
+  errorInputs.forEach(({ source, base }) => {
+    t.test(source, (t) => {
+      t.plan(1)
+      t.throws(() => buildURL(source, base))
+    })
+  })
+})