Skip to content

Commit 4d9795c

Browse files
rozzillamcollinaEomm
authored
Merge commit from fork
* advisory fix 1 Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * cleanup Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * update Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * update Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * . Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * Update test/fix-GHSA-2q7r-29rg-6m5h.js Signed-off-by: Matteo Collina <matteo.collina@gmail.com> * Update lib/utils.js Co-authored-by: Manuel Spigolon <manuel.spigolon@nearform.com> Signed-off-by: Roberto Bianchi <rbianchidev@gmail.com> * chore(test): add one Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * test * update Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * update Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> * chore: more tests * chore: fix host-header.test --------- Signed-off-by: Roberto Bianchi <roberto.bianchi@spendesk.com> Signed-off-by: Matteo Collina <matteo.collina@gmail.com> Signed-off-by: Roberto Bianchi <rbianchidev@gmail.com> Co-authored-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Manuel Spigolon <manuel.spigolon@nearform.com>
1 parent 79ff1d2 commit 4d9795c

File tree

5 files changed

+128
-1
lines changed

5 files changed

+128
-1
lines changed

index.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,7 +71,9 @@ const fastifyReplyFrom = fp(function from (fastify, opts, next) {
7171
const retryDelay = opts.retryDelay || undefined
7272

7373
if (!source) {
74-
source = req.url
74+
const requestUrl = req.url
75+
const queryIndex = requestUrl.indexOf('?')
76+
source = queryIndex >= 0 ? requestUrl.substring(0, queryIndex) : requestUrl
7577
}
7678

7779
// we leverage caching to avoid parsing the destination URL

lib/utils.js

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,12 @@ function stripHttp1ConnectionHeaders (headers) {
6363

6464
// issue ref: https://github.com/fastify/fast-proxy/issues/42
6565
function buildURL (source, reqBase) {
66+
if (decodeURIComponent(source).includes('..')) {
67+
const err = new Error('source/request contain invalid characters')
68+
err.statusCode = 400
69+
throw err
70+
}
71+
6672
if (Array.isArray(reqBase)) reqBase = reqBase[0]
6773
let baseOrigin = reqBase ? new URL(reqBase).href : undefined
6874

test/build-url.test.js

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,3 +76,35 @@ test('should throw when trying to override base', async (t) => {
7676

7777
await Promise.all(promises)
7878
})
79+
80+
test('should throw on path traversal attempts', (t) => {
81+
t.assert.throws(
82+
() => buildURL('/foo/bar/../', 'http://localhost'),
83+
new Error('source/request contain invalid characters')
84+
)
85+
86+
t.assert.throws(
87+
() => buildURL('/foo/bar/..', 'http://localhost'),
88+
new Error('source/request contain invalid characters')
89+
)
90+
91+
t.assert.throws(
92+
() => buildURL('/foo/bar/%2e%2e/', 'http://localhost'),
93+
new Error('source/request contain invalid characters')
94+
)
95+
96+
t.assert.throws(
97+
() => buildURL('/foo/bar/%2E%2E/', 'http://localhost'),
98+
new Error('source/request contain invalid characters')
99+
)
100+
101+
t.assert.throws(
102+
() => buildURL('/foo/bar/..%2f', 'http://localhost'),
103+
new Error('source/request contain invalid characters')
104+
)
105+
106+
t.assert.throws(
107+
() => buildURL('/foo/bar/%2e%2e%2f', 'http://localhost'),
108+
new Error('source/request contain invalid characters')
109+
)
110+
})
Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
'use strict'
2+
3+
const t = require('node:test')
4+
const Fastify = require('fastify')
5+
const { request } = require('undici')
6+
const From = require('..')
7+
const http = require('node:http')
8+
9+
const instance = Fastify()
10+
11+
t.test('fix for GHSA-2q7r-29rg-6m5h vulnerability', async (t) => {
12+
t.plan(2)
13+
14+
const target = http.createServer((_, res) => {
15+
res.statusCode = 205
16+
res.end('hi')
17+
})
18+
await target.listen({ port: 0 })
19+
t.after(() => target.close())
20+
21+
instance.get('/', (_request, reply) => { reply.from('/ho/%2E%2E/hi') })
22+
instance.register(From, {
23+
base: `http://localhost:${target.address().port}/hi/`,
24+
undici: true
25+
})
26+
await instance.listen({ port: 0 })
27+
t.after(() => instance.close())
28+
29+
const { statusCode, body } = await request(`http://localhost:${instance.server.address().port}`)
30+
t.assert.strictEqual(statusCode, 400)
31+
t.assert.strictEqual(await body.text(), '{"statusCode":400,"error":"Bad Request","message":"source/request contain invalid characters"}')
32+
})

test/full-querystring-url.test.js

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
'use strict'
2+
3+
const t = require('node:test')
4+
const Fastify = require('fastify')
5+
const { request } = require('undici')
6+
const From = require('..')
7+
const http = require('node:http')
8+
9+
const instance = Fastify()
10+
11+
t.test('full querystring url', async (t) => {
12+
const target = http.createServer((req, res) => {
13+
t.assert.ok('request proxied')
14+
t.assert.strictEqual(req.method, 'GET')
15+
t.assert.strictEqual(req.url, '/hi?a=/ho/%2E%2E/hi')
16+
res.statusCode = 205
17+
res.setHeader('Content-Type', 'text/plain')
18+
res.setHeader('x-my-header', 'hi!')
19+
res.end('hi')
20+
})
21+
22+
await target.listen({ port: 0 })
23+
t.after(() => target.close())
24+
25+
await instance.register(From, {
26+
base: `http://localhost:${target.address().port}`
27+
})
28+
29+
instance.get('/hi', (_request, reply) => {
30+
reply.from()
31+
})
32+
33+
instance.get('/foo', (_request, reply) => {
34+
reply.from('/hi')
35+
})
36+
37+
await instance.listen({ port: 0 })
38+
t.after(() => instance.close())
39+
40+
{
41+
const result = await request(`http://localhost:${instance.server.address().port}/hi?a=/ho/%2E%2E/hi`)
42+
t.assert.strictEqual(result.headers['content-type'], 'text/plain')
43+
t.assert.strictEqual(result.headers['x-my-header'], 'hi!')
44+
t.assert.strictEqual(result.statusCode, 205)
45+
t.assert.strictEqual(await result.body.text(), 'hi')
46+
}
47+
48+
{
49+
const result = await request(`http://localhost:${instance.server.address().port}/foo?a=/ho/%2E%2E/hi`)
50+
t.assert.strictEqual(result.headers['content-type'], 'text/plain')
51+
t.assert.strictEqual(result.headers['x-my-header'], 'hi!')
52+
t.assert.strictEqual(result.statusCode, 205)
53+
t.assert.strictEqual(await result.body.text(), 'hi')
54+
}
55+
})

0 commit comments

Comments
 (0)