Skip to content

Commit

Permalink
Enable prototype pollution protection in TSVB (#85952)
Browse files Browse the repository at this point in the history
* Enable prototype pollution protection in TSVB

Closes #78908

* Update Dock API Changes

* Replace logging failed in validateObject validation with 400 error

* Move validateObject to kbn-std package and add a description

* Update Doc API Changes

* Rename validateObject function to ensureNoUnsafeProperties

* Rename other validateObject occurrences

Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
  • Loading branch information
DianaDerevyankina and kibanamachine authored Dec 31, 2020
1 parent 602584a commit aecee92
Show file tree
Hide file tree
Showing 7 changed files with 17 additions and 27 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,14 @@
* under the License.
*/

import { validateObject } from './validate_object';
import { ensureNoUnsafeProperties } from './ensure_no_unsafe_properties';

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

expect(() =>
validateObject({
ensureNoUnsafeProperties({
payload: foo,
})
).toThrowErrorMatchingInlineSnapshot(`"circular reference detected"`);
Expand Down Expand Up @@ -57,7 +57,7 @@ test(`fails on circular references`, () => {
[property]: value,
};
test(`can submit ${JSON.stringify(obj)}`, () => {
expect(() => validateObject(obj)).not.toThrowError();
expect(() => ensureNoUnsafeProperties(obj)).not.toThrowError();
});
});
});
Expand All @@ -74,6 +74,6 @@ test(`fails on circular references`, () => {
JSON.parse(`{ "foo": { "bar": { "constructor": { "prototype" : null } } } }`),
].forEach((value) => {
test(`can't submit ${JSON.stringify(value)}`, () => {
expect(() => validateObject(value)).toThrowErrorMatchingSnapshot();
expect(() => ensureNoUnsafeProperties(value)).toThrowErrorMatchingSnapshot();
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const hasOwnProperty = (obj: any, property: string) =>
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) {
export function ensureNoUnsafeProperties(obj: any) {
if (!isObject(obj)) {
return;
}
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-std/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,5 @@ export { withTimeout } from './promise';
export { isRelativeUrl, modifyUrl, getUrlOrigin, URLMeaningfulParts } from './url';
export { unset } from './unset';
export { getFlattenedObject } from './get_flattened_object';
export { ensureNoUnsafeProperties } from './ensure_no_unsafe_properties';
export * from './rxjs_7';
4 changes: 2 additions & 2 deletions src/core/server/http/http_tools.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import Hoek from '@hapi/hoek';
import type { ServerOptions as TLSOptions } from 'https';
import type { ValidationError } from 'joi';
import uuid from 'uuid';
import { ensureNoUnsafeProperties } from '@kbn/std';
import { HttpConfig } from './http_config';
import { validateObject } from './prototype_pollution';

const corsAllowedHeaders = ['Accept', 'Authorization', 'Content-Type', 'If-None-Match', 'kbn-xsrf'];
/**
Expand Down Expand Up @@ -69,7 +69,7 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
// This is a default payload validation which applies to all LP routes which do not specify their own
// `validate.payload` handler, in order to reduce the likelyhood of prototype pollution vulnerabilities.
// (All NP routes are already required to specify their own validation in order to access the payload)
payload: (value) => Promise.resolve(validateObject(value)),
payload: (value) => Promise.resolve(ensureNoUnsafeProperties(value)),
},
},
state: {
Expand Down
20 changes: 0 additions & 20 deletions src/core/server/http/prototype_pollution/index.ts

This file was deleted.

9 changes: 9 additions & 0 deletions src/plugins/vis_type_timeseries/server/routes/vis.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@

import { IRouter, KibanaRequest } from 'kibana/server';
import { schema } from '@kbn/config-schema';
import { ensureNoUnsafeProperties } from '@kbn/std';
import { getVisData, GetVisDataOptions } from '../lib/get_vis_data';
import { visPayloadSchema } from '../../common/vis_schema';
import { ROUTES } from '../../common/constants';
Expand All @@ -40,6 +41,14 @@ export const visDataRoutes = (
},
},
async (requestContext, request, response) => {
try {
ensureNoUnsafeProperties(request.body);
} catch (error) {
return response.badRequest({
body: error.message,
});
}

try {
visPayloadSchema.validate(request.body);
} catch (error) {
Expand Down

0 comments on commit aecee92

Please sign in to comment.