Skip to content

Commit d0f7d63

Browse files
authored
fix: harden proxy-table matching to prevent routing bypass (#1268)
1 parent 617a7c9 commit d0f7d63

5 files changed

Lines changed: 73 additions & 9 deletions

File tree

CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
11
# Changelog
22

3+
## next
4+
5+
- fix(router): harden proxy-table matching (exact host for host+path keys, prefix-only path matching) to prevent routing bypass
6+
37
## [v2.0.9](https://github.com/chimurai/http-proxy-middleware/releases/tag/v2.0.9)
48

59
- fix(fixRequestBody): check readableLength

cspell.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
"rawbody",
3434
"restream",
3535
"streamify",
36+
"superstring",
3637
"vhosted",
3738
"websockets",
3839
"xfwd"

src/router.ts

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

1818
function getTargetFromProxyTable(req, table) {
1919
let result;
20-
const host = req.headers.host;
21-
const path = req.url;
22-
23-
const hostAndPath = host + path;
20+
const host = req.headers.host || '';
21+
const path = req.url || '';
2422

2523
for (const [key] of Object.entries(table)) {
2624
if (containsPath(key)) {
27-
if (hostAndPath.indexOf(key) > -1) {
28-
// match 'localhost:3000/api'
29-
result = table[key];
30-
logger.debug('[HPM] Router table match: "%s"', key);
31-
break;
25+
if (isHostAndPathKey(key)) {
26+
const [keyHost, keyPath] = splitHostAndPathKey(key);
27+
28+
// SECURITY: host+path keys must match exact host + path prefix.
29+
if (host === keyHost && path.startsWith(keyPath)) {
30+
// match 'localhost:3000/api'
31+
result = table[key];
32+
logger.debug('[HPM] Router table match: "%s"', key);
33+
break;
34+
}
35+
} else {
36+
if (path.startsWith(key)) {
37+
// match '/api'
38+
result = table[key];
39+
logger.debug('[HPM] Router table match: "%s"', key);
40+
break;
41+
}
3242
}
3343
} else {
3444
if (key === host) {
@@ -46,3 +56,12 @@ function getTargetFromProxyTable(req, table) {
4656
function containsPath(v) {
4757
return v.indexOf('/') > -1;
4858
}
59+
60+
function isHostAndPathKey(v) {
61+
return containsPath(v) && !v.startsWith('/');
62+
}
63+
64+
function splitHostAndPathKey(v) {
65+
const firstSlash = v.indexOf('/');
66+
return [v.slice(0, firstSlash), v.slice(firstSlash)];
67+
}

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 { createProxyMiddleware, createApp, createAppWithPath } from './test-kit';
23
import { ErrorRequestHandler } from 'express';
34
import * as request from 'supertest';
@@ -190,10 +191,22 @@ describe('E2E router', () => {
190191
expect(response.text).toBe('B');
191192
});
192193

194+
it('should not proxy host-only target when host is a crafted superstring', async () => {
195+
const response = await agent.get('/api').set('host', 'evilbeta.localhost:6000').expect(200);
196+
197+
expect(response.text).toBe('A');
198+
});
199+
193200
it('should proxy with host & path config: "localhost:6000/api"', async () => {
194201
const response = await agent.get('/api').set('host', 'localhost:6000').expect(200);
195202

196203
expect(response.text).toBe('C');
197204
});
205+
206+
it('should not proxy to host+path target when host is a crafted superstring', async () => {
207+
const response = await agent.get('/api').set('host', 'evillocalhost:6000').expect(200);
208+
209+
expect(response.text).toBe('A');
210+
});
198211
});
199212
});

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)