Skip to content

Commit de2576d

Browse files
authored
fix(router): harden proxy-table matching
1 parent f6be2c6 commit de2576d

5 files changed

Lines changed: 70 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
- fix(types): fix Logger type
66
- fix(error-response-plugin): sanitize input
7+
- fix(router): harden proxy-table matching (exact host for host+path keys, prefix-only path matching) to prevent routing bypass
78

89
## [v3.0.5](https://github.com/chimurai/http-proxy-middleware/releases/tag/v3.0.5)
910

cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@
4747
"restream",
4848
"snyk",
4949
"streamify",
50+
"superstring",
5051
"trivago",
5152
"tseslint",
5253
"typicode",

src/router.ts

Lines changed: 28 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -19,18 +19,28 @@ export async function getTarget(req, config) {
1919

2020
function getTargetFromProxyTable(req, table) {
2121
let result;
22-
const host = req.headers.host;
23-
const path = req.url;
24-
25-
const hostAndPath = host + path;
22+
const host = req.headers.host || '';
23+
const path = req.url || '';
2624

2725
for (const [key, value] of Object.entries(table)) {
2826
if (containsPath(key)) {
29-
if (hostAndPath.indexOf(key) > -1) {
30-
// match 'localhost:3000/api'
31-
result = value;
32-
debug('match: "%s" -> "%s"', key, result);
33-
break;
27+
if (isHostAndPathKey(key)) {
28+
const [keyHost, keyPath] = splitHostAndPathKey(key);
29+
30+
// SECURITY: host+path keys must match exact host + path prefix.
31+
if (host === keyHost && path.startsWith(keyPath)) {
32+
// match 'localhost:3000/api'
33+
result = value;
34+
debug('match: "%s" -> "%s"', key, result);
35+
break;
36+
}
37+
} else {
38+
if (path.startsWith(key)) {
39+
// match '/api'
40+
result = value;
41+
debug('match: "%s" -> "%s"', key, result);
42+
break;
43+
}
3444
}
3545
} else {
3646
if (key === host) {
@@ -48,3 +58,12 @@ function getTargetFromProxyTable(req, table) {
4858
function containsPath(v) {
4959
return v.indexOf('/') > -1;
5060
}
61+
62+
function isHostAndPathKey(v) {
63+
return containsPath(v) && !v.startsWith('/');
64+
}
65+
66+
function splitHostAndPathKey(v) {
67+
const firstSlash = v.indexOf('/');
68+
return [v.slice(0, firstSlash), v.slice(firstSlash)];
69+
}

test/e2e/router.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
/* spellchecker: ignore evilbeta, evillocalhost */
12
import { ErrorRequestHandler } from 'express';
23
import * as getPort from 'get-port';
34
import { Mockttp, generateCACertificate, getLocal } from 'mockttp';
@@ -185,10 +186,22 @@ describe('E2E router', () => {
185186
expect(response.text).toBe('B');
186187
});
187188

189+
it('should not proxy host-only target when host is a crafted superstring', async () => {
190+
const response = await agent.get('/api').set('host', 'evilbeta.localhost:6000').expect(200);
191+
192+
expect(response.text).toBe('A');
193+
});
194+
188195
it('should proxy with host & path config: "localhost:6000/api"', async () => {
189196
const response = await agent.get('/api').set('host', 'localhost:6000').expect(200);
190197

191198
expect(response.text).toBe('C');
192199
});
200+
201+
it('should not proxy to host+path target when host is a crafted superstring', async () => {
202+
const response = await agent.get('/api').set('host', 'evillocalhost:6000').expect(200);
203+
204+
expect(response.text).toBe('A');
205+
});
193206
});
194207
});

test/unit/router.spec.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
// spell-checker: ignore evilalpha, evilgamma
12
import { getTarget } from '../../src/router';
23

34
describe('router unit test', () => {
@@ -106,6 +107,12 @@ describe('router unit test', () => {
106107
result = getTarget(fakeReq, proxyOptionWithRouter);
107108
return expect(result).resolves.toBe('http://localhost:6002');
108109
});
110+
111+
it('should not match host-only config when host contains key as substring', () => {
112+
fakeReq.headers.host = 'evilalpha.localhost';
113+
result = getTarget(fakeReq, proxyOptionWithRouter);
114+
return expect(result).resolves.toBeUndefined();
115+
});
109116
});
110117

111118
describe('with host and host + path config', () => {
@@ -128,6 +135,20 @@ describe('router unit test', () => {
128135
result = getTarget(fakeReq, proxyOptionWithRouter);
129136
return expect(result).resolves.toBe('http://localhost:6003');
130137
});
138+
139+
it('should not match host+path config when host is a superstring', () => {
140+
fakeReq.headers.host = 'evilgamma.localhost';
141+
fakeReq.url = '/api';
142+
result = getTarget(fakeReq, proxyOptionWithRouter);
143+
return expect(result).resolves.toBeUndefined();
144+
});
145+
146+
it('should not match host+path config when host only contains host as a substring', () => {
147+
fakeReq.headers.host = 'gamma.localhost.evil';
148+
fakeReq.url = '/api/books/123';
149+
result = getTarget(fakeReq, proxyOptionWithRouter);
150+
return expect(result).resolves.toBeUndefined();
151+
});
131152
});
132153

133154
describe('with just the path', () => {
@@ -148,6 +169,12 @@ describe('router unit test', () => {
148169
result = getTarget(fakeReq, proxyOptionWithRouter);
149170
return expect(result).resolves.toBeUndefined();
150171
});
172+
173+
it('should not match path config when key appears as non-prefix substring', () => {
174+
fakeReq.url = '/prefix/rest';
175+
result = getTarget(fakeReq, proxyOptionWithRouter);
176+
return expect(result).resolves.toBeUndefined();
177+
});
151178
});
152179

153180
describe('matching order of router config', () => {

0 commit comments

Comments
 (0)