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 29, 2020
1 parent fb3694b commit a781485
Show file tree
Hide file tree
Showing 4 changed files with 146 additions and 3 deletions.
93 changes: 93 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,93 @@
/********************************************************************************
* 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.an('undefined');
expect(result.my.key1.foo).to.equal('value1');
const prototypeObject = Object.prototype as any;
expect(prototypeObject.injectedParsedPrototype).to.be.an('undefined');
const rawObject = {} as any;
expect(rawObject.injectedParsedPrototype).to.be.an('undefined');
});

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

});
9 changes: 7 additions & 2 deletions 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 injectionRe = /\b__proto__\b|\bconstructor\.prototype\b/;

enum ConfigurationTarget {
Global = 1,
Workspace = 2,
Expand Down Expand Up @@ -239,6 +241,9 @@ export class PreferenceRegistryExtImpl implements PreferenceRegistryExt {

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

Expand All @@ -248,7 +253,7 @@ export class PreferenceRegistryExtImpl implements PreferenceRegistryExt {
continue;
}
if (!branch[parts[i]]) {
branch[parts[i]] = {};
branch[parts[i]] = Object.create(null);
}
branch = branch[parts[i]];

Expand All @@ -263,7 +268,7 @@ export class PreferenceRegistryExtImpl implements PreferenceRegistryExt {
}
}
return result;
}, {});
}, Object.create(null));
}

private toConfigurationChangeEvent(eventData: PreferenceChangeExt[]): theia.ConfigurationChangeEvent {
Expand Down
42 changes: 42 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,45 @@ 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');
});

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

});

});
5 changes: 4 additions & 1 deletion packages/plugin-ext/src/plugin/preferences/configuration.ts
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ export class Configuration {
export class ConfigurationModel {

constructor(
private contents: any = {},
private contents: any = Object.create(null),
private keys: string[] = [],
) { }

Expand Down 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 a781485

Please sign in to comment.