Skip to content
Permalink
Browse files Browse the repository at this point in the history
Merge pull request from GHSA-qmw8-3v4g-gwj4
* fix: Re-enable unix+http:// base URL, prevent escape of base URL path

Co-Authored-By: Corey Farrell <git@cfware.com>

* More fixes

* Test more build-url error cases

Co-authored-by: Corey Farrell <git@cfware.com>
  • Loading branch information
mcollina and coreyfarrell committed Feb 19, 2021
1 parent 20d0795 commit dea227d
Show file tree
Hide file tree
Showing 3 changed files with 32 additions and 7 deletions.
10 changes: 8 additions & 2 deletions lib/utils.js
Expand Up @@ -59,8 +59,14 @@ 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')
if (reqBase) {
if (!reqBase.endsWith('/') && dest.href.length > reqBase.length) {
reqBase = reqBase + '/'
}

if (!dest.href.startsWith(reqBase)) {
throw new Error('source must be a relative path string')
}
}

return dest
Expand Down
20 changes: 19 additions & 1 deletion test/build-url.js
Expand Up @@ -21,13 +21,31 @@ test('should return same source when base is not specified', (t) => {
t.equal(url.href, 'http://localhost/hi')
})

test('should handle lack of trailing slash in base', (t) => {
t.plan(3)
let url = buildURL('hi', 'http://localhost/hi')
t.equal(url.href, 'http://localhost/hi')

url = buildURL('hi/', 'http://localhost/hi')
t.equal(url.href, 'http://localhost/hi/')

url = buildURL('hi/more', 'http://localhost/hi')
t.equal(url.href, 'http://localhost/hi/more')
})

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' }
{ source: 'urn:foo:bar', base: 'http://localhost' },
{ source: 'http://localhost/private', base: 'http://localhost/exposed/' },
{ source: 'http://localhost/exposed-extra', base: 'http://localhost/exposed' },
{ source: '/private', base: 'http://localhost/exposed/' },
{ source: '/exposed-extra', base: 'http://localhost/exposed' },
{ source: '../private', base: 'http://localhost/exposed/' },
{ source: 'exposed-extra', base: 'http://localhost/exposed' }
]

test('should throw when trying to override base', (t) => {
Expand Down
9 changes: 5 additions & 4 deletions test/unix-http.js
Expand Up @@ -13,14 +13,15 @@ if (process.platform === 'win32') {
process.exit(0)
}

const socketPath = `${__filename}.socket`
const upstream = `unix+http://${querystring.escape(socketPath)}/`

const instance = Fastify()
instance.register(From)
instance.register(From, { base: upstream })

t.plan(10)
t.tearDown(instance.close.bind(instance))

const socketPath = `${__filename}.socket`

try {
fs.unlinkSync(socketPath)
} catch (_) {
Expand All @@ -37,7 +38,7 @@ const target = http.createServer((req, res) => {
})

instance.get('/', (request, reply) => {
reply.from(`unix+http://${querystring.escape(socketPath)}/hello`)
reply.from('hello')
})

t.tearDown(target.close.bind(target))
Expand Down

0 comments on commit dea227d

Please sign in to comment.