Skip to content

Commit 4a2e457

Browse files
authored
fix(repeater): preserve path traversal vulnerability (#143)
closes #142
1 parent 7e82a1a commit 4a2e457

File tree

10 files changed

+278
-728
lines changed

10 files changed

+278
-728
lines changed

package-lock.json

Lines changed: 39 additions & 645 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -85,8 +85,6 @@
8585
"content-type": "^1.0.4",
8686
"form-data": "^4.0.0",
8787
"reflect-metadata": "^0.1.13",
88-
"request": "^2.88.2",
89-
"request-promise": "^4.2.6",
9088
"semver": "^7.3.7",
9189
"socks-proxy-agent": "^6.2.0-beta.0",
9290
"tslib": "~2.3.1",

packages/repeater/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
export { RepeatersManager } from './api';
2+
export { ExecuteRequestEventHandler } from './bus';
23
export * from './lib';
34
export * from './models';
45
export * from './request-runner';

packages/repeater/src/models/index.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,2 @@
11
export * from './Protocol';
22
export * from './RepeaterStatus';
3-
4-
export const Container: unique symbol = Symbol('DependencyContainer');

packages/repeater/src/request-runner/Request.spec.ts

Lines changed: 16 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -1,21 +1,24 @@
11
import { Request } from './Request';
2+
import { Protocol } from '../models';
23

34
describe('Request', () => {
45
describe('constructor', () => {
56
it('should throw Error on empty url', () => {
67
expect(
78
() =>
89
new Request({
9-
url: ''
10+
url: '',
11+
protocol: Protocol.HTTP
1012
})
11-
).toThrow('Url must be declared explicitly.');
13+
).toThrow('Invalid URL.');
1214
});
1315

1416
it('should throw Error on invalid url', () => {
1517
expect(
1618
() =>
1719
new Request({
18-
url: 'http::/foo.bar'
20+
url: 'http::/foo.bar',
21+
protocol: Protocol.HTTP
1922
})
2023
).toThrow('Invalid URL.');
2124
});
@@ -25,7 +28,8 @@ describe('Request', () => {
2528
() =>
2629
new Request({
2730
url: 'http://foo.bar',
28-
body: 42 as unknown as string
31+
body: 42 as unknown as string,
32+
protocol: Protocol.HTTP
2933
})
3034
).toThrow('Body must be string.');
3135
});
@@ -35,7 +39,8 @@ describe('Request', () => {
3539
() =>
3640
new Request({
3741
url: 'http://foo.bar',
38-
correlationIdRegex: '('
42+
correlationIdRegex: '(',
43+
protocol: Protocol.HTTP
3944
})
4045
).toThrow('Correlation id must be regular expression.');
4146
});
@@ -44,7 +49,8 @@ describe('Request', () => {
4449
expect(
4550
() =>
4651
new Request({
47-
url: 'http://foo.bar'
52+
url: 'http://foo.bar',
53+
protocol: Protocol.HTTP
4854
})
4955
).not.toThrow();
5056
});
@@ -55,28 +61,19 @@ describe('Request', () => {
5561
expect(
5662
new Request({
5763
url: 'http://foo.bar',
58-
method: 'post'
64+
method: 'post',
65+
protocol: Protocol.HTTP
5966
}).method
6067
).toBe('POST');
6168
});
6269
});
6370

64-
describe('url', () => {
65-
it('should normalize url', () => {
66-
expect(
67-
new Request({
68-
url: 'HTTP://foo.BAR',
69-
method: 'post'
70-
}).url
71-
).toBe('http://foo.bar/');
72-
});
73-
});
74-
7571
describe('setHeaders', () => {
7672
it('should append headers', () => {
7773
const request = new Request({
7874
url: 'http://foo.bar',
79-
headers: { 'x-key': 'value' }
75+
headers: { 'x-key': 'value' },
76+
protocol: Protocol.HTTP
8077
});
8178

8279
request.setHeaders({ 'x-a1': 'a1', 'x-a2': 'a2' });

packages/repeater/src/request-runner/Request.ts

Lines changed: 18 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,17 @@
1+
import { Protocol } from '../models';
12
import { URL } from 'url';
23

34
export interface RequestOptions {
4-
method?: string;
5+
protocol: Protocol;
56
url: string;
6-
headers: Record<string, string | string[]>;
7+
method?: string;
8+
headers?: Record<string, string | string[]>;
79
body?: string;
810
correlationIdRegex?: string | RegExp;
911
}
1012

1113
export class Request {
14+
public readonly protocol: Protocol;
1215
public readonly url: string;
1316
public readonly body?: string;
1417
public readonly correlationIdRegex?: RegExp;
@@ -19,27 +22,31 @@ export class Request {
1922
return this._method;
2023
}
2124

22-
private _headers: Record<string, string | string[]>;
25+
private _headers?: Record<string, string | string[]>;
2326

24-
get headers(): Readonly<Record<string, string | string[]>> {
27+
get headers(): Readonly<Record<string, string | string[]>> | undefined {
2528
return this._headers;
2629
}
2730

31+
get secureEndpoint(): boolean {
32+
return this.url.startsWith('https');
33+
}
34+
2835
constructor({
36+
protocol,
2937
method,
3038
url,
3139
body,
3240
correlationIdRegex,
3341
headers = {}
34-
}: Omit<RequestOptions, 'headers'> & {
35-
headers?: Record<string, string | string[]>;
36-
}) {
42+
}: RequestOptions) {
43+
this.protocol = protocol;
3744
this._method = method?.toUpperCase() ?? 'GET';
38-
this.url = this.normalizeUrl(url);
45+
this.validateUrl(url);
46+
this.url = url;
3947
this.correlationIdRegex =
4048
this.normalizeCorrelationIdRegex(correlationIdRegex);
4149
this._headers = headers;
42-
4350
this.precheckBody(body);
4451
this.body = body;
4552
}
@@ -51,13 +58,9 @@ export class Request {
5158
};
5259
}
5360

54-
private normalizeUrl(url: string): string {
55-
if (!url) {
56-
throw new Error('Url must be declared explicitly.');
57-
}
58-
61+
private validateUrl(url: string): void {
5962
try {
60-
return new URL(url).toString();
63+
new URL(url);
6164
} catch {
6265
throw new Error('Invalid URL.');
6366
}

packages/repeater/src/request-runner/protocols/HttpRequestRunner.spec.ts

Lines changed: 36 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,14 +7,14 @@ import 'reflect-metadata';
77
import { anything, spy, verify, when } from 'ts-mockito';
88
import { Logger, LogLevel } from '@sectester/core';
99
import { SocksProxyAgent } from 'socks-proxy-agent';
10-
import { constants, gzip } from 'zlib';
10+
import { brotliCompress, constants, gzip } from 'zlib';
1111
import { promisify } from 'util';
1212

1313
const createRequest = (options: Partial<RequestOptions> = {}) => {
1414
const requestOptions = {
15+
protocol: Protocol.HTTP,
1516
url: 'https://foo.bar',
1617
method: 'GET',
17-
headers: {},
1818
...options
1919
};
2020
const request = new Request(requestOptions);
@@ -84,6 +84,22 @@ describe('HttpRequestRunner', () => {
8484
});
8585
});
8686

87+
it('should preserve directory traversal', async () => {
88+
const runner = setupRunner();
89+
const path = 'public/../../../../../../etc/passwd';
90+
const { request } = createRequest({
91+
url: `http://localhost:8080/${path}`
92+
});
93+
nock('http://localhost:8080').get(`/${path}`).reply(200, {});
94+
95+
const response = await runner.run(request);
96+
97+
expect(response).toMatchObject({
98+
statusCode: 200,
99+
body: {}
100+
});
101+
});
102+
87103
it('should handle HTTP errors', async () => {
88104
const runner = setupRunner();
89105
const { request, requestOptions } = createRequest();
@@ -158,6 +174,24 @@ describe('HttpRequestRunner', () => {
158174
expect(response.body).toEqual(expected);
159175
});
160176

177+
it('should decode response body if content-encoding is brotli', async () => {
178+
const runner = setupRunner({
179+
maxContentLength: 1,
180+
allowedMimes: ['text/plain']
181+
});
182+
const { request, requestOptions } = createRequest();
183+
const expected = 'x'.repeat(1025);
184+
const bigBody = await promisify(brotliCompress)(expected);
185+
nock(requestOptions.url).get('/').reply(200, bigBody, {
186+
'content-type': 'text/plain',
187+
'content-encoding': 'br'
188+
});
189+
190+
const response = await runner.run(request);
191+
192+
expect(response.body).toEqual(expected);
193+
});
194+
161195
it('should decode and truncate gzipped response body if content-type is not in allowed list', async () => {
162196
const runner = setupRunner({
163197
maxContentLength: 1,

0 commit comments

Comments
 (0)