Skip to content

Commit ecf6af5

Browse files
gaborzoltanbediyesoreyeram
authored
fixes (#407)
* Fix: Check method to be POST or GET * Fix: Do not allow .. in URL * Remove unnecessary variable * Fix test * fix async problem in tests * move isSafeURL to a deeper level * lint fix * more complex check * preparing release 1.3.11 * Update package.json * Update cspell.config.json * prepare cloud release (#6) --------- Co-authored-by: Zoltán Bedi <zoltan.bedi@gmail.com> Co-authored-by: Sriram <153843+yesoreyeram@users.noreply.github.com>
1 parent 8b11290 commit ecf6af5

File tree

4 files changed

+80
-1
lines changed

4 files changed

+80
-1
lines changed

cspell.config.json

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
"./.github/",
55
"./dist/",
66
"coverage",
7+
"package.json",
78
"./src/**/*.test.ts",
89
"./website/src/css",
910
"./website/*.js",

src/api.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -148,6 +148,28 @@ export default class Api {
148148
.join('&');
149149
}
150150

151+
// it is important to validate here, directly before the request is sent out,
152+
// because, for example, template-variables could be used to adjust the final url, etc.
153+
if (!isSafeURL(req.url)) {
154+
throw new Error('URL path contains unsafe characters');
155+
}
156+
151157
return getBackendSrv().fetch(req);
152158
}
153159
}
160+
161+
function isSafeURL(url: string) {
162+
if (url.endsWith('/..')) {
163+
return false;
164+
}
165+
166+
if (url.includes('/../')) {
167+
return false;
168+
}
169+
170+
if (url.includes('/..?')) {
171+
return false;
172+
}
173+
174+
return true;
175+
}

src/datasource.test.ts

Lines changed: 53 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,21 @@
11
import { dateTime, TimeRange } from '@grafana/data';
2-
import { replaceMacros } from './datasource';
2+
import { JsonDataSource, replaceMacros } from './datasource';
3+
4+
jest.mock('@grafana/runtime', () => ({
5+
...jest.requireActual('@grafana/runtime'),
6+
getTemplateSrv: () => ({
7+
getVariables: () => [],
8+
replace: (text?: string) => text,
9+
}),
10+
getBackendSrv: () => ({
11+
fetch: () => ({
12+
toPromise: () =>
13+
Promise.resolve({
14+
data: [{ id: 1 }, { id: 2 }, { id: 3 }],
15+
}),
16+
}),
17+
}),
18+
}));
319

420
const sampleTimestampFrom = '2021-05-17T20:48:09.000Z'; // -> 1621284489
521
const sampleTimestmapTo = '2021-05-17T20:50:23.000Z'; // -> 1621284623
@@ -22,3 +38,39 @@ test('range gets converted into unix epoch notation', () => {
2238
expect(replaceMacros('$__unixEpochFrom()', range)).toStrictEqual('1621284489');
2339
expect(replaceMacros('$__unixEpochTo()', range)).toStrictEqual('1621284623');
2440
});
41+
42+
describe('datasource', () => {
43+
it('should not allow urls that contains ..', async () => {
44+
const ds = new JsonDataSource({ url: 'http://localhost:3000', jsonData: {} } as any);
45+
const badPaths = ['/..', '/..?', '/../../', '/../..?'];
46+
47+
for (let path of badPaths) {
48+
const response = ds.doRequest({ urlPath: path, method: 'GET' } as any);
49+
await expect(response).rejects.toThrowError('URL path contains unsafe characters');
50+
}
51+
52+
const goodPaths = ['/..thing', '/one..two/', '/thing../'];
53+
54+
for (let path of goodPaths) {
55+
const response = ds.doRequest({ urlPath: path, method: 'GET' } as any);
56+
// i just need to check that there were no errors, so i check for the response-length
57+
await expect(response).resolves.toHaveLength(1);
58+
}
59+
});
60+
61+
it('should throw error when method is not POST or GET', async () => {
62+
const ds = new JsonDataSource({ url: 'http://localhost:3000', jsonData: {} } as any);
63+
64+
const responsePUT = ds.doRequest({ method: 'PUT' } as any);
65+
66+
await expect(responsePUT).rejects.toThrowError('Invalid method PUT');
67+
68+
const responsePATCH = ds.doRequest({ method: 'PATCH' } as any);
69+
70+
await expect(responsePATCH).rejects.toThrowError('Invalid method PATCH');
71+
72+
const responseDELETE = ds.doRequest({ method: 'DELETE' } as any);
73+
74+
await expect(responseDELETE).rejects.toThrowError('Invalid method DELETE');
75+
});
76+
});

src/datasource.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -237,6 +237,10 @@ export class JsonDataSource extends DataSourceApi<JsonApiQuery, JsonApiDataSourc
237237
return [interpolate(key), interpolate(value)];
238238
};
239239

240+
if (query.method !== 'GET' && query.method !== 'POST') {
241+
throw new Error(`Invalid method ${query.method}`);
242+
}
243+
240244
return await this.api.cachedGet(
241245
query.cacheDurationSeconds,
242246
query.method,

0 commit comments

Comments
 (0)