Default payload validation (#48753)

* trial for default payload validation

* relaxing default validation

* some cleanup and testing

* update xsrf integration test

* adding API smoke tests

* fixing types

* removing Joi extensions

* updating tests

* documenting changes

* fixing NP validation bypass

* fix lint problems

* Update src/legacy/server/http/integration_tests/xsrf.test.js

* Update src/legacy/server/http/integration_tests/xsrf.test.js

* revert test changes

* simplifying tests


Co-authored-by: Elastic Machine <elasticmachine@users.noreply.github.com>

src/core/server/http/base_path_proxy_server.ts

@@ -143,6 +143,7 @@ export class BasePathProxyServer {
             return responseToolkit.continue;
           },
         ],
+        validate: { payload: true },
       },
       path: `${this.httpConfig.basePath}/{kbnPath*}`,
     });
@@ -175,6 +176,7 @@ export class BasePathProxyServer {
             return responseToolkit.continue;
           },
         ],
+        validate: { payload: true },
       },
       path: `/__UNSAFE_bypassBasePath/{kbnPath*}`,
     });

src/core/server/http/http_server.ts

@@ -128,6 +128,8 @@ export class HttpServer {
       for (const route of router.getRoutes()) {
         this.log.debug(`registering route handler for [${route.path}]`);
         const { authRequired = true, tags } = route.options;
+        // Hapi does not allow payload validation to be specified for 'head' or 'get' requests
+        const validate = ['head', 'get'].includes(route.method) ? undefined : { payload: true };
         this.server.route({
           handler: route.handler,
           method: route.method,
@@ -135,6 +137,11 @@ export class HttpServer {
           options: {
             auth: authRequired ? undefined : false,
             tags: tags ? Array.from(tags) : undefined,
+            // TODO: This 'validate' section can be removed once the legacy platform is completely removed.
+            // We are telling Hapi that NP routes can accept any payload, so that it can bypass the default
+            // validation applied in ./http_tools#getServerOptions
+            // (All NP routes are already required to specify their own validation in order to access the payload)
+            validate,
           },
         });
       }

src/core/server/http/http_tools.ts

@@ -23,6 +23,7 @@ import Hoek from 'hoek';
 import { ServerOptions as TLSOptions } from 'https';
 import { ValidationError } from 'joi';
 import { HttpConfig } from './http_config';
+import { validateObject } from './prototype_pollution';
 
 /**
  * Converts Kibana `HttpConfig` into `ServerOptions` that are accepted by the Hapi server.
@@ -45,6 +46,11 @@ export function getServerOptions(config: HttpConfig, { configureTLS = true } = {
         options: {
           abortEarly: false,
         },
+        // TODO: This payload validation can be removed once the legacy platform is completely removed.
+        // 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)),
       },
     },
     state: {

src/core/server/http/prototype_pollution/__snapshots__/validate_object.test.ts.snap

@@ -0,0 +1,13 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`can't submit {"__proto__":null} 1`] = `"'__proto__' is an invalid key"`;
+
+exports[`can't submit {"constructor":{"prototype":null}} 1`] = `"'constructor.prototype' is an invalid key"`;
+
+exports[`can't submit {"foo":{"__proto__":true}} 1`] = `"'__proto__' is an invalid key"`;
+
+exports[`can't submit {"foo":{"bar":{"__proto__":{}}}} 1`] = `"'__proto__' is an invalid key"`;
+
+exports[`can't submit {"foo":{"bar":{"constructor":{"prototype":null}}}} 1`] = `"'constructor.prototype' is an invalid key"`;
+
+exports[`can't submit {"foo":{"constructor":{"prototype":null}}} 1`] = `"'constructor.prototype' is an invalid key"`;

src/core/server/http/prototype_pollution/index.ts

@@ -0,0 +1,20 @@
+/*
+ * 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.
+ */
+
+export { validateObject } from './validate_object';

src/core/server/http/prototype_pollution/validate_object.test.ts

@@ -0,0 +1,79 @@
+/*
+ * 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();
+  });
+});

src/core/server/http/prototype_pollution/validate_object.ts

@@ -0,0 +1,80 @@
+/*
+ * 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.
+ */
+
+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,
+      });
+    }
+  }
+}

src/legacy/core_plugins/console/server/proxy_route.js

@@ -71,6 +71,7 @@ export const createProxyRoute = ({
       parse: false,
     },
     validate: {
+      payload: true,
       query: Joi.object()
         .keys({
           method: Joi.string()

src/legacy/server/http/integration_tests/xsrf.test.js

@@ -57,7 +57,8 @@ describe('xsrf request filter', () => {
         // Disable payload parsing to make HapiJS server accept any content-type header.
         payload: {
           parse: false
-        }
+        },
+        validate: { payload: null }
       },
       handler: async function () {
         return 'ok';
@@ -71,7 +72,8 @@ describe('xsrf request filter', () => {
         // Disable payload parsing to make HapiJS server accept any content-type header.
         payload: {
           parse: false
-        }
+        },
+        validate: { payload: null }
       },
       handler: async function () {
         return 'ok';

test/api_integration/apis/general/index.js

@@ -21,5 +21,6 @@ export default function ({ loadTestFile }) {
   describe('general', () => {
     loadTestFile(require.resolve('./cookies'));
     loadTestFile(require.resolve('./csp'));
+    loadTestFile(require.resolve('./prototype_pollution'));
   });
 }

test/api_integration/apis/general/prototype_pollution.ts

@@ -0,0 +1,57 @@
+/*
+ * 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 { FtrProviderContext } from 'test/api_integration/ftr_provider_context';
+
+// eslint-disable-next-line import/no-default-export
+export default function({ getService }: FtrProviderContext) {
+  const supertest = getService('supertest');
+
+  describe('prototype pollution smoke test', () => {
+    it('prevents payloads with the "constructor.prototype" pollution vector from being accepted', async () => {
+      await supertest
+        .post('/api/sample_data/some_data_id')
+        .send([
+          {
+            constructor: {
+              prototype: 'foo',
+            },
+          },
+        ])
+        .expect(400, {
+          statusCode: 400,
+          error: 'Bad Request',
+          message: "'constructor.prototype' is an invalid key",
+          validation: { source: 'payload', keys: [] },
+        });
+    });
+
+    it('prevents payloads with the "__proto__" pollution vector from being accepted', async () => {
+      await supertest
+        .post('/api/sample_data/some_data_id')
+        .send(JSON.parse(`{"foo": { "__proto__": {} } }`))
+        .expect(400, {
+          statusCode: 400,
+          error: 'Bad Request',
+          message: "'__proto__' is an invalid key",
+          validation: { source: 'payload', keys: [] },
+        });
+    });
+  });
+}

test/api_integration/ftr_provider_context.d.ts

@@ -0,0 +1,24 @@
+/*
+ * 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 { GenericFtrProviderContext } from '@kbn/test/types/ftr';
+
+import { services } from './services';
+
+export type FtrProviderContext = GenericFtrProviderContext<typeof services, {}>;