Enable prototype pollution protection in TSVB (#85952) (#87087)

* 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> Co-authored-by: Diana Derevyankina <54894989+DziyanaDzeraviankina@users.noreply.github.com> Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
elastic · Dec 31, 2020 · 69c4eb2 · 69c4eb2
1 parent 78f6d9a
commit 69c4eb2
Show file tree

Hide file tree

Showing 7 changed files with 17 additions and 27 deletions.
diff --git a/..._snapshots__/validate_object.test.ts.snap → .../ensure_no_unsafe_properties.test.ts.snap b/..._snapshots__/validate_object.test.ts.snap → .../ensure_no_unsafe_properties.test.ts.snap
diff --git a/...ototype_pollution/validate_object.test.ts → ...d/src/ensure_no_unsafe_properties.test.ts b/...ototype_pollution/validate_object.test.ts → ...d/src/ensure_no_unsafe_properties.test.ts
@@ -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"`);
@@ -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();
     });
   });
 });
@@ -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();
   });
 });
diff --git a/...tp/prototype_pollution/validate_object.ts → ...bn-std/src/ensure_no_unsafe_properties.ts b/...tp/prototype_pollution/validate_object.ts → ...bn-std/src/ensure_no_unsafe_properties.ts
@@ -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;
   }

diff --git a/packages/kbn-std/src/index.ts b/packages/kbn-std/src/index.ts
@@ -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';
diff --git a/src/core/server/http/http_tools.ts b/src/core/server/http/http_tools.ts
@@ -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'];
 /**
@@ -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: {

diff --git a/src/core/server/http/prototype_pollution/index.ts b/src/core/server/http/prototype_pollution/index.ts
diff --git a/src/plugins/vis_type_timeseries/server/routes/vis.ts b/src/plugins/vis_type_timeseries/server/routes/vis.ts
@@ -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';
@@ -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) {