Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[CVE] Handle invalid query, index and date in vega charts filter handlers #1932

Merged
merged 3 commits into from
Jul 28, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/osd-std/src/__snapshots__/validate_object.test.ts.snap

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/osd-std/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,5 @@ export { withTimeout } from './promise';
export { isRelativeUrl, modifyUrl, getUrlOrigin, URLMeaningfulParts } from './url';
export { unset } from './unset';
export { getFlattenedObject } from './get_flattened_object';
export { validateObject } from './validate_object';
export * from './rxjs_7';
90 changes: 90 additions & 0 deletions packages/osd-std/src/validate_object.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,90 @@
/*
bandinib-amzn marked this conversation as resolved.
Show resolved Hide resolved
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Any modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch B.V. under one or more contributor
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

import { validateObject } from './validate_object';

test(`fails on circular references`, () => {
const foo: Record<string, any> = {};
foo.myself = foo;

expect(() =>
validateObject({
payload: foo,
})
).toThrowErrorMatchingInlineSnapshot(`"circular reference detected"`);
});

[
{
foo: true,
bar: '__proto__',
baz: 1.1,
qux: undefined,
quux: () => null,
quuz: Object.create(null),
},
{
foo: {
foo: true,
bar: '__proto__',
baz: 1.1,
qux: undefined,
quux: () => null,
quuz: Object.create(null),
},
},
{ constructor: { foo: { prototype: null } } },
{ prototype: { foo: { constructor: null } } },
].forEach((value) => {
['headers', 'payload', 'query', 'params'].forEach((property) => {
const obj = {
[property]: value,
};
test(`can submit ${JSON.stringify(obj)}`, () => {
expect(() => validateObject(obj)).not.toThrowError();
});
});
});

// if we use the object literal syntax to create the following values, we end up
// actually reassigning the __proto__ which makes it be a non-enumerable not-own property
// which isn't what we want to test here
[
JSON.parse(`{ "__proto__": null }`),
JSON.parse(`{ "foo": { "__proto__": true } }`),
JSON.parse(`{ "foo": { "bar": { "__proto__": {} } } }`),
JSON.parse(`{ "constructor": { "prototype" : null } }`),
JSON.parse(`{ "foo": { "constructor": { "prototype" : null } } }`),
JSON.parse(`{ "foo": { "bar": { "constructor": { "prototype" : null } } } }`),
].forEach((value) => {
test(`can't submit ${JSON.stringify(value)}`, () => {
expect(() => validateObject(value)).toThrowErrorMatchingSnapshot();
});
});
91 changes: 91 additions & 0 deletions packages/osd-std/src/validate_object.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Any modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch B.V. under one or more contributor
bandinib-amzn marked this conversation as resolved.
Show resolved Hide resolved
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

interface StackItem {
value: any;
previousKey: string | null;
}

// we have to do Object.prototype.hasOwnProperty because when you create an object using
// Object.create(null), and I assume other methods, you get an object without a prototype,
// so you can't use current.hasOwnProperty
const hasOwnProperty = (obj: any, property: string) =>
Object.prototype.hasOwnProperty.call(obj, property);

const isObject = (obj: any) => typeof obj === 'object' && obj !== null;

// we're using a stack instead of recursion so we aren't limited by the call stack
export function validateObject(obj: any) {
if (!isObject(obj)) {
return;
}

const stack: StackItem[] = [
{
value: obj,
previousKey: null,
},
];
const seen = new WeakSet([obj]);

while (stack.length > 0) {
const { value, previousKey } = stack.pop()!;

if (!isObject(value)) {
continue;
}

if (hasOwnProperty(value, '__proto__')) {
throw new Error(`'__proto__' is an invalid key`);
}

if (hasOwnProperty(value, 'prototype') && previousKey === 'constructor') {
throw new Error(`'constructor.prototype' is an invalid key`);
}

// iterating backwards through an array is reportedly more performant
const entries = Object.entries(value);
for (let i = entries.length - 1; i >= 0; --i) {
const [key, childValue] = entries[i];
if (isObject(childValue)) {
if (seen.has(childValue)) {
throw new Error('circular reference detected');
}

seen.add(childValue);
}

stack.push({
value: childValue,
previousKey: key,
});
}
}
}
118 changes: 118 additions & 0 deletions src/plugins/vis_type_vega/public/data_model/utils.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
/*
* SPDX-License-Identifier: Apache-2.0
*
* The OpenSearch Contributors require contributions made to
* this file be licensed under the Apache-2.0 license or a
* compatible open source license.
*
* Any modifications Copyright OpenSearch Contributors. See
* GitHub history for details.
*/

/*
* Licensed to Elasticsearch B.V. under one or more contributor
bandinib-amzn marked this conversation as resolved.
Show resolved Hide resolved
* license agreements. See the NOTICE file distributed with
* this work for additional information regarding copyright
* ownership. Elasticsearch B.V. licenses this file to you under
* the Apache License, Version 2.0 (the "License"); you may
* not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/
import { Utils } from './utils';

describe('Utils.handleNonStringIndex', () => {
test('should return same string on string input', async () => {
expect(Utils.handleNonStringIndex('*')).toBe('*');
});

test('should return empty string on empty string input', async () => {
expect(Utils.handleNonStringIndex('')).toBe('');
});

test('should return undefined on non-string input', async () => {
expect(Utils.handleNonStringIndex(123)).toBe(undefined);
expect(Utils.handleNonStringIndex(null)).toBe(undefined);
expect(Utils.handleNonStringIndex(undefined)).toBe(undefined);
});
});

describe('Utils.handleInvalidDate', () => {
test('should return null if passed timestamp is Not A Number', async () => {
expect(Utils.handleInvalidDate(Number.NaN)).toBe(null);
});

test('should return timestamp if passed timestamp is valid Number', async () => {
expect(Utils.handleInvalidDate(1658189958487)).toBe(1658189958487);
});

test('should return string on string input', async () => {
expect(Utils.handleInvalidDate('Sat Jul 16 2022 22:59:42')).toBe('Sat Jul 16 2022 22:59:42');
});

test('should return date on date input', async () => {
const date = Date.now();
expect(Utils.handleInvalidDate(date)).toBe(date);
});

test('should return null if input neigther timestamp, nor date, nor string', async () => {
expect(Utils.handleInvalidDate(undefined)).toBe(null);
expect(Utils.handleInvalidDate({ key: 'value' })).toBe(null);
});
});

describe('Utils.handleInvalidQuery', () => {
test('should return valid object on valid DSL query', async () => {
const testQuery = { match_phrase: { customer_gender: 'MALE' } };
expect(Utils.handleInvalidQuery(testQuery)).toStrictEqual(testQuery);
});

test('should return null on null or undefined input', async () => {
expect(Utils.handleInvalidQuery(null)).toBe(null);
expect(Utils.handleInvalidQuery(undefined)).toBe(null);
});

test('should return null if input object has function as property', async () => {
const input = {
key1: 'value1',
key2: () => {
alert('Hello!');
},
};

expect(Utils.handleInvalidQuery(input)).toBe(null);
});

test('should return null if nested object has function as property', async () => {
const input = {
key1: 'value1',
key2: {
func: () => {
alert('Hello!');
},
},
};
expect(Utils.handleInvalidQuery(input)).toBe(null);
});

test('should throw error on polluted query', async () => {
const maliciousQueries = [
JSON.parse(`{ "__proto__": null }`),
JSON.parse(`{ "constructor": { "prototype" : null } }`),
];

maliciousQueries.forEach((value) => {
expect(() => {
Utils.handleInvalidQuery(value);
}).toThrowError();
});
});
});
38 changes: 38 additions & 0 deletions src/plugins/vis_type_vega/public/data_model/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
*/

import compactStringify from 'json-stringify-pretty-compact';
import { validateObject } from '@osd/std';

export class Utils {
/**
Expand Down Expand Up @@ -59,4 +60,41 @@ export class Utils {
}
return Utils.formatWarningToStr(error, ...Array.from(args).slice(1));
}

static handleNonStringIndex(index: unknown): string | undefined {
return typeof index === 'string' ? index : undefined;
}

static handleInvalidQuery(query: unknown): object | null {
if (Utils.isObject(query) && !Utils.checkForFunctionProperty(query as object)) {
// Validating object against prototype pollution
validateObject(query);
return JSON.parse(JSON.stringify(query));
}
return null;
}

static isObject(object: unknown): boolean {
return !!(object && typeof object === 'object' && !Array.isArray(object));
}

static checkForFunctionProperty(object: object): boolean {
let result = false;
Object.values(object).forEach((value) => {
result =
typeof value === 'function'
? true
: Utils.isObject(value) && Utils.checkForFunctionProperty(value);
});
return result;
}

static handleInvalidDate(date: unknown): number | string | Date | null {
if (typeof date === 'number') {
return !isNaN(date) ? date : null;
} else if (date instanceof Date || typeof date === 'string') {
return date;
}
return null;
}
}
Loading