Skip to content

Commit 73211f7

Browse files
authored
Merge pull request #900 from rluvaton/master
fix possible RCE in jsonpath-plus
2 parents f215e3d + 0d12672 commit 73211f7

File tree

5 files changed

+51
-6
lines changed

5 files changed

+51
-6
lines changed

src/azure_auth.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import * as proc from 'child_process';
22
import https = require('https');
3-
import * as jsonpath from 'jsonpath-plus';
43
import request = require('request');
54

65
import { Authenticator } from './auth';
76
import { User } from './config_types';
7+
import { jsonpath } from './json_path';
88

99
/* FIXME: maybe we can extend the User and User.authProvider type to have a proper type.
1010
Currently user.authProvider has `any` type and so we don't have a type for user.authProvider.config.
@@ -94,7 +94,7 @@ export class AzureAuth implements Authenticator {
9494
const tokenPathKey = '$' + tokenPathKeyInConfig.slice(1, -1);
9595
const expiryPathKey = '$' + expiryPathKeyInConfig.slice(1, -1);
9696

97-
config['access-token'] = jsonpath.JSONPath(tokenPathKey, resultObj);
98-
config.expiry = jsonpath.JSONPath(expiryPathKey, resultObj);
97+
config['access-token'] = jsonpath(tokenPathKey, resultObj);
98+
config.expiry = jsonpath(expiryPathKey, resultObj);
9999
}
100100
}

src/gcp_auth.ts

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
import * as proc from 'child_process';
22
import https = require('https');
3-
import * as jsonpath from 'jsonpath-plus';
43
import request = require('request');
54

65
import { Authenticator } from './auth';
76
import { User } from './config_types';
7+
import { jsonpath } from './json_path';
88

99
/* FIXME: maybe we can extend the User and User.authProvider type to have a proper type.
1010
Currently user.authProvider has `any` type and so we don't have a type for user.authProvider.config.
@@ -90,7 +90,7 @@ export class GoogleCloudPlatformAuth implements Authenticator {
9090
const tokenPathKey = '$' + tokenPathKeyInConfig.slice(1, -1);
9191
const expiryPathKey = '$' + expiryPathKeyInConfig.slice(1, -1);
9292

93-
config['access-token'] = jsonpath.JSONPath(tokenPathKey, resultObj);
94-
config.expiry = jsonpath.JSONPath(expiryPathKey, resultObj);
93+
config['access-token'] = jsonpath(tokenPathKey, resultObj);
94+
config.expiry = jsonpath(expiryPathKey, resultObj);
9595
}
9696
}

src/gcp_auth_test.ts

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -285,4 +285,27 @@ describe('GoogleCloudPlatformAuth', () => {
285285
expect(opts.headers.Authorization).to.equal(`Bearer ${token}`);
286286
}
287287
});
288+
289+
it('should throw if tried to run JavaScript inside the token key', async () => {
290+
const token = 'token';
291+
const responseStr = `{"token":{"accessToken":"${token}"}}`;
292+
293+
const user = {
294+
authProvider: {
295+
name: 'gcp',
296+
config: {
297+
'cmd-path': join(__dirname, '..', 'test', 'echo space.js'),
298+
'cmd-args': `'${responseStr}'`,
299+
'expiry-key': '{.token.token_expiry}',
300+
301+
// The problematic token
302+
'token-key': '{..[?(' + '(function a(arr){' + 'a([...arr, ...arr])' + '})([1]);)]}',
303+
},
304+
},
305+
} as User;
306+
307+
await expect(auth.applyAuthentication(user, {})).to.eventually.be.rejectedWith(
308+
'Eval [?(expr)] prevented in JSONPath expression.',
309+
);
310+
});
288311
});

src/json_path.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
import { JSONPath } from 'jsonpath-plus';
2+
3+
export function jsonpath(path: string, json: object): any {
4+
return JSONPath({
5+
path,
6+
json,
7+
8+
preventEval: true,
9+
});
10+
}

src/json_path_test.ts

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import { expect } from 'chai';
2+
import { jsonpath } from './json_path';
3+
4+
describe('jsonpath', () => {
5+
it('should throw if tried to run JavaScript inside the path', () => {
6+
expect(() => {
7+
jsonpath('$..[?(' + '(function a(arr){' + 'a([...arr, ...arr])' + '})([1]);)]', {
8+
nonEmpty: 'object',
9+
});
10+
}).to.throw();
11+
});
12+
});

0 commit comments

Comments
 (0)