CVE-2022-35949: Server-Side Request Forgery (SSRF) (CWE-918)
value: undici is an HTTP/1.1 client, written from scratch for Node.js.`undici` is vulnerable to SSRF (Server-side Request Forgery) when an application takes in **user input** into the `path/pathname` option of `undici.request`. If a user specifies a URL such as `http://127.0.0.1` or `//127.0.0.1` ```js const undici = require("undici") undici.request({origin: "http://example.com", pathname: "//127.0.0.1"}) ``` Instead of processing the request as `http://example.org//127.0.0.1` (or `http://example.org/http://127.0.0.1` when `http://127.0.0.1 is used`), it actually processes the request as `http://127.0.0.1/` and sends it to `http://127.0.0.1`. If a developer passes in user input into `path` parameter of `undici.request`, it can result in an _SSRF_ as they will assume that the hostname cannot change, when in actual fact it can change because the specified path parameter is combined with the base URL. This issue was fixed in `undici@5.8.1`. The best workaround is to validate user input before passing it to the `undici.request` call.
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: 9.8
Published: 2022-08-12T23:15Z
Repository: https://github.com/nodejs/undici
Language: JavaScript
Files changed: index.js, lib/core/util.js, test/request.js
Commit date: 2022-08-09 11:29:11 +0200
Commit message: Merge pull request from GHSA-8qr4-xgw6-wmr3


Diff: diff --git a/index.js b/index.js
index b2144c8..8099f5a 100644
--- a/index.js
+++ b/index.js
@@ -51,11 +51,16 @@ function makeDispatcher (fn) {
     if (opts && opts.path != null) {
       if (typeof opts.path !== 'string') {
         throw new InvalidArgumentError('invalid opts.path')
       }
 
-      url = new URL(opts.path, util.parseOrigin(url))
+      let path = opts.path
+      if (!opts.path.startsWith('/')) {
+        path = `/${path}`
+      }
+
+      url = new URL(util.parseOrigin(url).origin + path)
     } else {
       if (!opts) {
         opts = typeof url === 'object' ? url : {}
       }
 
diff --git a/lib/core/util.js b/lib/core/util.js
index 635ef2e..5b0c5d1 100644
--- a/lib/core/util.js
+++ b/lib/core/util.js
@@ -106,18 +106,29 @@ function parseURL (url) {
 
   if (!(url instanceof URL)) {
     const port = url.port != null
       ? url.port
       : (url.protocol === 'https:' ? 443 : 80)
-    const origin = url.origin != null
+    let origin = url.origin != null
       ? url.origin
       : `${url.protocol}//${url.hostname}:${port}`
-    const path = url.path != null
+    let path = url.path != null
       ? url.path
       : `${url.pathname || ''}${url.search || ''}`
 
-    url = new URL(path, origin)
+    if (origin.endsWith('/')) {
+      origin = origin.substring(0, origin.length - 1)
+    }
+
+    if (path && !path.startsWith('/')) {
+      path = `/${path}`
+    }
+    // new URL(path, origin) is unsafe when `path` contains an absolute URL
+    // From https://developer.mozilla.org/en-US/docs/Web/API/URL/URL:
+    // If first parameter is a relative URL, second param is required, and will be used as the base URL.
+    // If first parameter is an absolute URL, a given second param will be ignored.
+    url = new URL(origin + path)
   }
 
   return url
 }
 
diff --git a/test/request.js b/test/request.js
new file mode 100644
index 0000000..52e3ecf
--- /dev/null
+++ b/test/request.js
@@ -0,0 +1,151 @@
+'use strict'
+
+const { createServer } = require('http')
+const { test } = require('tap')
+const { request } = require('..')
+
+test('no-slash/one-slash pathname should be included in req.path', async (t) => {
+  const pathServer = createServer((req, res) => {
+    t.fail('it shouldn\'t be called')
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  const requestedServer = createServer((req, res) => {
+    t.equal(`/localhost:${pathServer.address().port}`, req.url)
+    t.equal('GET', req.method)
+    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  t.teardown(requestedServer.close.bind(requestedServer))
+  t.teardown(pathServer.close.bind(pathServer))
+
+  await Promise.all([
+    requestedServer.listen(0),
+    pathServer.listen(0)
+  ])
+
+  const noSlashPathname = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    pathname: `localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPathname.statusCode, 200)
+  const noSlashPath = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    path: `localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPath.statusCode, 200)
+  const noSlashPath2Arg = await request(
+    `http://localhost:${requestedServer.address().port}`,
+    { path: `localhost:${pathServer.address().port}` }
+  )
+  t.equal(noSlashPath2Arg.statusCode, 200)
+  const oneSlashPathname = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    pathname: `/localhost:${pathServer.address().port}`
+  })
+  t.equal(oneSlashPathname.statusCode, 200)
+  const oneSlashPath = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    path: `/localhost:${pathServer.address().port}`
+  })
+  t.equal(oneSlashPath.statusCode, 200)
+  const oneSlashPath2Arg = await request(
+    `http://localhost:${requestedServer.address().port}`,
+    { path: `/localhost:${pathServer.address().port}` }
+  )
+  t.equal(oneSlashPath2Arg.statusCode, 200)
+  t.end()
+})
+
+test('protocol-relative URL as pathname should be included in req.path', async (t) => {
+  const pathServer = createServer((req, res) => {
+    t.fail('it shouldn\'t be called')
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  const requestedServer = createServer((req, res) => {
+    t.equal(`//localhost:${pathServer.address().port}`, req.url)
+    t.equal('GET', req.method)
+    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  t.teardown(requestedServer.close.bind(requestedServer))
+  t.teardown(pathServer.close.bind(pathServer))
+
+  await Promise.all([
+    requestedServer.listen(0),
+    pathServer.listen(0)
+  ])
+
+  const noSlashPathname = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    pathname: `//localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPathname.statusCode, 200)
+  const noSlashPath = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    path: `//localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPath.statusCode, 200)
+  const noSlashPath2Arg = await request(
+    `http://localhost:${requestedServer.address().port}`,
+    { path: `//localhost:${pathServer.address().port}` }
+  )
+  t.equal(noSlashPath2Arg.statusCode, 200)
+  t.end()
+})
+
+test('Absolute URL as pathname should be included in req.path', async (t) => {
+  const pathServer = createServer((req, res) => {
+    t.fail('it shouldn\'t be called')
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  const requestedServer = createServer((req, res) => {
+    t.equal(`/http://localhost:${pathServer.address().port}`, req.url)
+    t.equal('GET', req.method)
+    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
+    res.statusCode = 200
+    res.end('hello')
+  })
+
+  t.teardown(requestedServer.close.bind(requestedServer))
+  t.teardown(pathServer.close.bind(pathServer))
+
+  await Promise.all([
+    requestedServer.listen(0),
+    pathServer.listen(0)
+  ])
+
+  const noSlashPathname = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    pathname: `http://localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPathname.statusCode, 200)
+  const noSlashPath = await request({
+    method: 'GET',
+    origin: `http://localhost:${requestedServer.address().port}`,
+    path: `http://localhost:${pathServer.address().port}`
+  })
+  t.equal(noSlashPath.statusCode, 200)
+  const noSlashPath2Arg = await request(
+    `http://localhost:${requestedServer.address().port}`,
+    { path: `http://localhost:${pathServer.address().port}` }
+  )
+  t.equal(noSlashPath2Arg.statusCode, 200)
+  t.end()
+})

Vulnerable code:       url = new URL(opts.path, util.parseOrigin(url))
    const origin = url.origin != null
    const path = url.path != null
    url = new URL(path, origin)
Fixed code:       let path = opts.path
      if (!opts.path.startsWith('/')) {
        path = `/${path}`
      }

      url = new URL(util.parseOrigin(url).origin + path)
    let origin = url.origin != null
    let path = url.path != null
    if (origin.endsWith('/')) {
      origin = origin.substring(0, origin.length - 1)
    }

    if (path && !path.startsWith('/')) {
      path = `/${path}`
    }
    // new URL(path, origin) is unsafe when `path` contains an absolute URL
    // From https://developer.mozilla.org/en-US/docs/Web/API/URL/URL:
    // If first parameter is a relative URL, second param is required, and will be used as the base URL.
    // If first parameter is an absolute URL, a given second param will be ignored.
    url = new URL(origin + path)
'use strict'

const { createServer } = require('http')
const { test } = require('tap')
const { request } = require('..')

test('no-slash/one-slash pathname should be included in req.path', async (t) => {
  const pathServer = createServer((req, res) => {
    t.fail('it shouldn\'t be called')
    res.statusCode = 200
    res.end('hello')
  })

  const requestedServer = createServer((req, res) => {
    t.equal(`/localhost:${pathServer.address().port}`, req.url)
    t.equal('GET', req.method)
    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
    res.statusCode = 200
    res.end('hello')
  })

  t.teardown(requestedServer.close.bind(requestedServer))
  t.teardown(pathServer.close.bind(pathServer))

  await Promise.all([
    requestedServer.listen(0),
    pathServer.listen(0)
  ])

  const noSlashPathname = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    pathname: `localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPathname.statusCode, 200)
  const noSlashPath = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    path: `localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPath.statusCode, 200)
  const noSlashPath2Arg = await request(
    `http://localhost:${requestedServer.address().port}`,
    { path: `localhost:${pathServer.address().port}` }
  )
  t.equal(noSlashPath2Arg.statusCode, 200)
  const oneSlashPathname = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    pathname: `/localhost:${pathServer.address().port}`
  })
  t.equal(oneSlashPathname.statusCode, 200)
  const oneSlashPath = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    path: `/localhost:${pathServer.address().port}`
  })
  t.equal(oneSlashPath.statusCode, 200)
  const oneSlashPath2Arg = await request(
    `http://localhost:${requestedServer.address().port}`,
    { path: `/localhost:${pathServer.address().port}` }
  )
  t.equal(oneSlashPath2Arg.statusCode, 200)
  t.end()
})

test('protocol-relative URL as pathname should be included in req.path', async (t) => {
  const pathServer = createServer((req, res) => {
    t.fail('it shouldn\'t be called')
    res.statusCode = 200
    res.end('hello')
  })

  const requestedServer = createServer((req, res) => {
    t.equal(`//localhost:${pathServer.address().port}`, req.url)
    t.equal('GET', req.method)
    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
    res.statusCode = 200
    res.end('hello')
  })

  t.teardown(requestedServer.close.bind(requestedServer))
  t.teardown(pathServer.close.bind(pathServer))

  await Promise.all([
    requestedServer.listen(0),
    pathServer.listen(0)
  ])

  const noSlashPathname = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    pathname: `//localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPathname.statusCode, 200)
  const noSlashPath = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    path: `//localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPath.statusCode, 200)
  const noSlashPath2Arg = await request(
    `http://localhost:${requestedServer.address().port}`,
    { path: `//localhost:${pathServer.address().port}` }
  )
  t.equal(noSlashPath2Arg.statusCode, 200)
  t.end()
})

test('Absolute URL as pathname should be included in req.path', async (t) => {
  const pathServer = createServer((req, res) => {
    t.fail('it shouldn\'t be called')
    res.statusCode = 200
    res.end('hello')
  })

  const requestedServer = createServer((req, res) => {
    t.equal(`/http://localhost:${pathServer.address().port}`, req.url)
    t.equal('GET', req.method)
    t.equal(`localhost:${requestedServer.address().port}`, req.headers.host)
    res.statusCode = 200
    res.end('hello')
  })

  t.teardown(requestedServer.close.bind(requestedServer))
  t.teardown(pathServer.close.bind(pathServer))

  await Promise.all([
    requestedServer.listen(0),
    pathServer.listen(0)
  ])

  const noSlashPathname = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    pathname: `http://localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPathname.statusCode, 200)
  const noSlashPath = await request({
    method: 'GET',
    origin: `http://localhost:${requestedServer.address().port}`,
    path: `http://localhost:${pathServer.address().port}`
  })
  t.equal(noSlashPath.statusCode, 200)
  const noSlashPath2Arg = await request(
    `http://localhost:${requestedServer.address().port}`,
    { path: `http://localhost:${pathServer.address().port}` }
  )
  t.equal(noSlashPath2Arg.statusCode, 200)
  t.end()
})