Skip to content

Commit

Permalink
fix(merge): Avoid protototype pollution when parsing properties
Browse files Browse the repository at this point in the history
Change-Id: I30ac10c9afce8a6fe01e197e18071e33f0e0bda7
Signed-off-by: Florent Benoit <[email protected]>
  • Loading branch information
benoitf authored and paul-marechal committed Oct 28, 2020
1 parent e07af50 commit 8cccac3
Show file tree
Hide file tree
Showing 4 changed files with 121 additions and 1 deletion.
83 changes: 83 additions & 0 deletions packages/plugin-ext/src/plugin/preference-registry.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/********************************************************************************
* Copyright (C) 2020 Red Hat, Inc. and others.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0.
*
* This Source Code may also be made available under the following Secondary
* Licenses when the conditions for such availability set forth in the Eclipse
* Public License v. 2.0 are satisfied: GNU General Public License, version 2
* with the GNU Classpath Exception which is available at
* https://www.gnu.org/software/classpath/license.html.
*
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

import { PreferenceRegistryExtImpl } from './preference-registry';
import * as chai from 'chai';
import { WorkspaceExtImpl } from '../plugin/workspace';
import { RPCProtocol } from '../common/rpc-protocol';
import { ProxyIdentifier } from '../common/rpc-protocol';

const expect = chai.expect;

/* eslint-disable @typescript-eslint/no-explicit-any */
describe('PreferenceRegistryExtImpl:', () => {

let preferenceRegistryExtImpl: PreferenceRegistryExtImpl;
const getProxy = (proxyId: ProxyIdentifier<unknown>) => { };
const set = (identifier: ProxyIdentifier<unknown>, instance: unknown) => { };
const dispose = () => { };

const mockRPC = {
getProxy,
set,
dispose

} as RPCProtocol;
const mockWorkspace: WorkspaceExtImpl = {} as WorkspaceExtImpl;

beforeEach(() => {
preferenceRegistryExtImpl = new PreferenceRegistryExtImpl(mockRPC, mockWorkspace);
});

it('Check parseConfigurationData', () => {
const value: { [key: string]: any } = {
'my.key1.foo': 'value1',
'my.key1.bar': 'value2',
};
const result: { [key: string]: any } = (preferenceRegistryExtImpl as any).parseConfigurationData(value);
expect(result.my).to.be.an('object');
expect(result.my.key1).to.be.an('object');

expect(result.my.key1.foo).to.be.an('string');
expect(result.my.key1.foo).to.equal('value1');

expect(result.my.key1.bar).to.be.an('string');
expect(result.my.key1.bar).to.equal('value2');
});

it('Prototype pollution check', () => {
const value: { [key: string]: any } = {
'my.key1.foo': 'value1',
'__proto__.injectedParsedPrototype': true,
'a.__proto__.injectedParsedPrototype': true,
'__proto__': {},
};
const result: { [key: string]: any } = (preferenceRegistryExtImpl as any).parseConfigurationData(value);
expect(result.my).to.be.an('object');

expect(result.__proto__).to.be.equal(Object.prototype);

expect(result.my.key1.foo).to.equal('value1');

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const prototypeObject = Object.prototype as any;
expect(prototypeObject.injectedParsedPrototype).to.be.an('undefined');
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const rawObject = {} as any;
expect(rawObject.injectedParsedPrototype).to.be.an('undefined');
});

});
4 changes: 3 additions & 1 deletion packages/plugin-ext/src/plugin/preference-registry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ import { Configuration, ConfigurationModel } from './preferences/configuration';
import { WorkspaceExtImpl } from './workspace';
import cloneDeep = require('lodash.clonedeep');

const prototypePropertyRe = /\b__proto__\b/;

enum ConfigurationTarget {
Global = 1,
Workspace = 2,
Expand Down Expand Up @@ -238,7 +240,7 @@ export class PreferenceRegistryExtImpl implements PreferenceRegistryExt {
private readonly OVERRIDE_PROPERTY_PATTERN = new RegExp(this.OVERRIDE_PROPERTY);

private parseConfigurationData(data: { [key: string]: any }): { [key: string]: any } {
return Object.keys(data).reduce((result: any, key: string) => {
return Object.keys(data).filter(key => !prototypePropertyRe.test(key)).reduce((result: any, key: string) => {
const parts = key.split('.');
let branch = result;

Expand Down
32 changes: 32 additions & 0 deletions packages/plugin-ext/src/plugin/preferences/configuration.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
* SPDX-License-Identifier: EPL-2.0 OR GPL-2.0 WITH Classpath-exception-2.0
********************************************************************************/

/* eslint-disable @typescript-eslint/no-explicit-any */
import * as chai from 'chai';
import { Configuration, ConfigurationModel } from './configuration';
import { PreferenceData } from '../../common';
Expand Down Expand Up @@ -247,4 +248,35 @@ describe('Configuration:', () => {

});

describe('ConfigurationModel', () => {

it('check merge', () => {
defaultConfiguration = new ConfigurationModel(
preferences[PreferenceScope.Default],
Object.keys(preferences[PreferenceScope.Default])
);
userConfiguration = new ConfigurationModel(
preferences[PreferenceScope.User],
Object.keys(preferences[PreferenceScope.User])
);
workspaceConfiguration = new ConfigurationModel(
preferences[PreferenceScope.Workspace],
Object.keys(preferences[PreferenceScope.Workspace])
);
const mergedConfiguration = new ConfigurationModel().merge(defaultConfiguration, userConfiguration, workspaceConfiguration);
expect(mergedConfiguration.getValue('tabSize')).to.equal(4);
});

it('Prototype pollution check', () => {
const payload = JSON.parse('{"__proto__":{"injectedConfigurationPrototype": true}}');
const configurationModel = new ConfigurationModel();
configurationModel.merge(new ConfigurationModel(payload));
const prototypeObject = Object.prototype as any;
expect(prototypeObject.injectedConfigurationPrototype).to.be.an('undefined');
const rawObject = {} as any;
expect(rawObject.injectedConfigurationPrototype).to.be.an('undefined');
});

});

});
3 changes: 3 additions & 0 deletions packages/plugin-ext/src/plugin/preferences/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,6 +143,9 @@ export class ConfigurationModel {

private mergeContents(source: any, target: any): void {
for (const key of Object.keys(target)) {
if (key === '__proto__') {
continue;
}
if (key in source) {
if (isObject(source[key]) && isObject(target[key])) {
this.mergeContents(source[key], target[key]);
Expand Down

0 comments on commit 8cccac3

Please sign in to comment.