Skip to content

Commit

Permalink
Additional prototype pollution protections (elastic#206073)
Browse files Browse the repository at this point in the history
## Summary

1. Extends the server-side prototype pollution protections introduced in
elastic#190716 to include
`Array.prototype`.
2. Applies the same prototype pollution protections to the client-side.


### Identify risks

Does this PR introduce any risks? For example, consider risks like hard
to test bugs, performance regression, potential of data loss.

Describe the risk, its severity, and mitigation for each identified
risk. Invite stakeholders and evaluate how to proceed before merging.

- [ ] Sealing prototypes on the client can lead to failures in
third-party dependencies. I'm relying on sufficient functional test
coverage to detect issues here. As a result, these protections are
disabled by default for now, and can be controlled via setting
`server.prototypeHardening: true/false`

---------

Co-authored-by: kibanamachine <[email protected]>
Co-authored-by: Elastic Machine <[email protected]>
  • Loading branch information
3 people authored Jan 28, 2025
1 parent dcf64f2 commit 9ce2dd8
Show file tree
Hide file tree
Showing 40 changed files with 360 additions and 86 deletions.
2 changes: 1 addition & 1 deletion .github/CODEOWNERS
Original file line number Diff line number Diff line change
Expand Up @@ -1868,7 +1868,7 @@ x-pack/platform/plugins/private/cloud_integrations/cloud_full_story/server/confi
/x-pack/test/ftr_apis/common/fixtures/es_archiver/base_data/space_1.json @elastic/kibana-security # Assigned per only use: https://github.com/elastic/kibana/blob/main/x-pack/test/ftr_apis/security_and_spaces/apis/test_utils.ts#L33
/x-pack/test/ftr_apis/common/fixtures/es_archiver/base_data/default_space.json @elastic/kibana-security # Assigned per only use: https://github.com/elastic/kibana/blob/main/x-pack/test/ftr_apis/security_and_spaces/apis/test_utils.ts#L33
/x-pack/test/api_integration/apis/cloud @elastic/kibana-security # Assigned per https://github.com/elastic/kibana/pull/198444
/test/plugin_functional/snapshots/baseline/hardening/prototype.json @elastic/kibana-security # Assigned per https://github.com/elastic/kibana/pull/190716
/test/plugin_functional/snapshots/baseline/hardening @elastic/kibana-security # Assigned per https://github.com/elastic/kibana/pull/190716
/test/functional/page_objects/login_page.ts @elastic/kibana-security
/x-pack/test_serverless/functional/test_suites/observability/role_management @elastic/kibana-security
/x-pack/test/functional/config_security_basic.ts @elastic/kibana-security
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -1220,7 +1220,7 @@
"react-diff-view": "^3.2.1",
"react-dom": "^17.0.2",
"react-dom-18": "npm:react-dom@~18.2.0",
"react-dropzone": "^4.2.9",
"react-dropzone": "^11.7.1",
"react-fast-compare": "^2.0.4",
"react-hook-form": "^7.44.2",
"react-intl": "6.6.6",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ const createInternalPrebootContractMock = (args: CreateMockArgs = {}) => {
basePath,
staticAssets: createInternalStaticAssetsMock(basePath, args.cdnUrl),
csp: CspConfig.DEFAULT,
prototypeHardening: false,
externalUrl: ExternalUrlConfig.DEFAULT,
auth: createAuthMock(),
getServerInfo: jest.fn(),
Expand Down Expand Up @@ -182,6 +183,7 @@ const createInternalSetupContractMock = () => {
registerStaticDir: jest.fn(),
basePath,
csp: CspConfig.DEFAULT,
prototypeHardening: false,
staticAssets: createInternalStaticAssetsMock(basePath),
externalUrl: ExternalUrlConfig.DEFAULT,
auth: createAuthMock(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,7 @@ export const defaultConfig: ScoutServerConfig = {
serverArgs: [
`--server.restrictInternalApis=true`,
`--server.port=${servers.kibana.port}`,
`--server.prototypeHardening=true`,
'--status.allowAnonymous=true',
`--migrations.zdt.runOnRoles=${JSON.stringify(['ui'])}`,
// We shouldn't embed credentials into the URL since Kibana requests to Elasticsearch should
Expand Down
1 change: 1 addition & 0 deletions packages/kbn-scout/src/config/stateful/base.config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ export const defaultConfig: ScoutServerConfig = {
sourceArgs: ['--no-base-path', '--env.name=development'],
serverArgs: [
`--server.port=${servers.kibana.port}`,
`--server.prototypeHardening=true`,
'--status.allowAnonymous=true',
// We shouldn't embed credentials into the URL since Kibana requests to Elasticsearch should
// either include `kibanaServerTestUser` credentials, or credentials provided by the test
Expand Down
2 changes: 2 additions & 0 deletions src/cli/serve/serve.js
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,8 @@ export function applyConfigOverrides(rawConfig, opts, extraCliOptions, keystoreC
);
}

set('server.prototypeHardening', true);

if (!has('elasticsearch.serviceAccountToken') && opts.devCredentials !== false) {
if (!has('elasticsearch.username')) {
set('elasticsearch.username', 'kibana_system');
Expand Down

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

10 changes: 10 additions & 0 deletions src/core/packages/http/server-internal/src/http_config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -676,6 +676,16 @@ describe('http2 protocol', () => {
});
});

describe('prototypeHardening', () => {
it('defaults to false', () => {
expect(config.schema.validate({}).prototypeHardening).toBe(false);
});

it('can be set to true', () => {
expect(config.schema.validate({ prototypeHardening: true }).prototypeHardening).toBe(true);
});
});

describe('HttpConfig', () => {
it('converts customResponseHeaders to strings or arrays of strings', () => {
const httpSchema = config.schema;
Expand Down
3 changes: 3 additions & 0 deletions src/core/packages/http/server-internal/src/http_config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,7 @@ const configSchema = schema.object(
defaultValue: 'http1',
})
),
prototypeHardening: schema.boolean({ defaultValue: false }),
host: schema.string({
defaultValue: 'localhost',
hostname: true,
Expand Down Expand Up @@ -354,6 +355,7 @@ export class HttpConfig implements IHttpConfig {
brotli: { enabled: boolean; quality: number };
};
public csp: ICspConfig;
public prototypeHardening: boolean;
public externalUrl: IExternalUrlConfig;
public xsrf: { disableProtection: boolean; allowlist: string[] };
public requestId: { allowFromAnyIp: boolean; ipAllowlist: string[] };
Expand Down Expand Up @@ -408,6 +410,7 @@ export class HttpConfig implements IHttpConfig {
this.compression = rawHttpConfig.compression;
this.cdn = CdnConfig.from(rawHttpConfig.cdn);
this.csp = new CspConfig({ ...rawCspConfig, disableEmbedding }, this.cdn.getCspConfig());
this.prototypeHardening = rawHttpConfig.prototypeHardening;
this.externalUrl = rawExternalUrlConfig;
this.xsrf = rawHttpConfig.xsrf;
this.requestId = rawHttpConfig.requestId;
Expand Down
2 changes: 2 additions & 0 deletions src/core/packages/http/server-internal/src/http_server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,6 +141,7 @@ export interface HttpServerSetup {
staticAssets: InternalStaticAssets;
basePath: HttpServiceSetup['basePath'];
csp: HttpServiceSetup['csp'];
prototypeHardening: boolean;
createCookieSessionStorageFactory: HttpServiceSetup['createCookieSessionStorageFactory'];
registerOnPreRouting: HttpServiceSetup['registerOnPreRouting'];
registerOnPreAuth: HttpServiceSetup['registerOnPreAuth'];
Expand Down Expand Up @@ -307,6 +308,7 @@ export class HttpServer {
),
basePath: basePathService,
csp: config.csp,
prototypeHardening: config.prototypeHardening,
auth: {
get: this.authState.get,
isAuthenticated: this.authState.isAuthenticated,
Expand Down
1 change: 1 addition & 0 deletions src/core/packages/http/server-internal/src/http_service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -129,6 +129,7 @@ export class HttpService
this.internalPreboot = {
externalUrl: new ExternalUrlConfig(config.externalUrl),
csp: prebootSetup.csp,
prototypeHardening: prebootSetup.prototypeHardening,
staticAssets: prebootSetup.staticAssets,
basePath: prebootSetup.basePath,
registerStaticDir: prebootSetup.registerStaticDir.bind(prebootSetup),
Expand Down
2 changes: 2 additions & 0 deletions src/core/packages/http/server-internal/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface InternalHttpServicePreboot
| 'registerRouteHandlerContext'
| 'server'
| 'getServerInfo'
| 'prototypeHardening'
> {
registerRoutes<
DefaultRequestHandlerType extends RequestHandlerContextBase = RequestHandlerContextBase
Expand All @@ -54,6 +55,7 @@ export interface InternalHttpServiceSetup
server: HttpServerSetup['server'];
staticAssets: InternalStaticAssets;
externalUrl: ExternalUrlConfig;
prototypeHardening: boolean;
createRouter: <Context extends RequestHandlerContextBase = RequestHandlerContextBase>(
path: string,
plugin?: PluginOpaqueId
Expand Down

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

Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ function kbnBundlesLoader() {
}
var kbnCsp = JSON.parse(document.querySelector('kbn-csp').getAttribute('data'));
var kbnHardenPrototypes = JSON.parse(document.querySelector('kbn-prototype-hardening').getAttribute('data'));
window.__kbnHardenPrototypes__ = kbnHardenPrototypes.hardenPrototypes;
window.__kbnStrictCsp__ = kbnCsp.strictCsp;
window.__kbnThemeTag__ = "${themeTag}";
window.__kbnPublicPath__ = ${publicPathMap};
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ export class RenderingService {
const bootstrapScript = isAnonymousPage ? 'bootstrap-anonymous.js' : 'bootstrap.js';
const metadata: RenderingMetadata = {
strictCsp: http.csp.strict,
hardenPrototypes: http.prototypeHardening,
uiPublicUrl: `${staticAssetsHrefBase}/ui`,
bootstrapScriptUrl: `${basePath}/${bootstrapScript}`,
locale,
Expand Down
1 change: 1 addition & 0 deletions src/core/packages/rendering/server-internal/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ import type { InternalFeatureFlagsSetup } from '@kbn/core-feature-flags-server-i

/** @internal */
export interface RenderingMetadata {
hardenPrototypes: ICspConfig['strict'];
strictCsp: ICspConfig['strict'];
uiPublicUrl: string;
bootstrapScriptUrl: string;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ export const Template: FunctionComponent<Props> = ({
scriptPaths,
injectedMetadata,
bootstrapScriptUrl,
hardenPrototypes,
strictCsp,
customBranding,
},
Expand Down Expand Up @@ -74,6 +75,9 @@ export const Template: FunctionComponent<Props> = ({
{createElement('kbn-csp', {
data: JSON.stringify({ strictCsp }),
})}
{createElement('kbn-prototype-hardening', {
data: JSON.stringify({ hardenPrototypes }),
})}
{createElement('kbn-injected-metadata', { data: JSON.stringify(injectedMetadata) })}
<div
className="kbnWelcomeView"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -165,6 +165,7 @@ kibana_vars=(
server.name
server.port
server.protocol
server.prototypeHardening
server.publicBaseUrl
server.requestId.allowFromAnyIp
server.requestId.ipAllowlist
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ webpack_cli(
"//src/platform/packages/shared/kbn-crypto-browser",
"//src/platform/packages/shared/kbn-es-query",
"//src/platform/packages/shared/kbn-search-errors",
"//src/platform/packages/shared/kbn-security-hardening",
"//src/platform/packages/shared/kbn-std",
"//src/platform/packages/shared/kbn-safer-lodash-set",
"//packages/kbn-peggy",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,15 @@
*/

require('./polyfills');
// Optional prototype hardening. This must occur immediately after polyfills.
if (typeof window.__kbnHardenPrototypes__ !== 'boolean') {
throw new Error(
'Invariant bootstrap failure: __kbnHardenPrototypes__ must be set to true or false'
);
}
if (window.__kbnHardenPrototypes__) {
require('@kbn/security-hardening/prototype');
}

export const Jquery = require('jquery');
window.$ = window.jQuery = Jquery;
Expand Down
32 changes: 32 additions & 0 deletions src/platform/packages/shared/kbn-security-hardening/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
load("@build_bazel_rules_nodejs//:index.bzl", "js_library")

SRCS = glob(
[
"**/*.ts",
],
exclude = [
"**/test_helpers.ts",
"**/*.config.js",
"**/*.mock.*",
"**/*.test.*",
"**/*.stories.*",
"**/__snapshots__/**",
"**/integration_tests/**",
"**/mocks/**",
"**/scripts/**",
"**/storybook/**",
"**/test_fixtures/**",
"**/test_helpers/**",
],
)

BUNDLER_DEPS = [
]

js_library(
name = "kbn-security-hardening",
package_name = "@kbn/security-hardening",
srcs = ["package.json"] + SRCS,
deps = BUNDLER_DEPS,
visibility = ["//visibility:public"],
)
Original file line number Diff line number Diff line change
Expand Up @@ -4,4 +4,8 @@ A package counterpart of `src/setup_node_env/harden` - containing overrides, uti

## console

When running in production mode (`process.env.NODE_ENV === 'production'`), global console methods `debug`, `error`, `info`, `log`, `trace`, and `warn` are overridden to implement input sanitization. The export `unsafeConsole` provides access to the unmodified global console methods.
When running in production mode (`process.env.NODE_ENV === 'production'`), global console methods `debug`, `error`, `info`, `log`, `trace`, and `warn` are overridden to implement input sanitization. The export `unsafeConsole` provides access to the unmodified global console methods.

## prototype

The prototypes of most built-in classes are sealed to mitigate many prototype pollution vulnerabilities.
Original file line number Diff line number Diff line change
Expand Up @@ -7,4 +7,5 @@
* License v3.0 only", or the "Server Side Public License, v 1".
*/

import './prototype';
export { unsafeConsole } from './console';
44 changes: 44 additions & 0 deletions src/platform/packages/shared/kbn-security-hardening/prototype.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

/**
* Harden the prototypes of built-in objects to prevent prototype pollution attacks.
* This function should be called after the polyfills have been loaded, as some polyfills require the prototypes to be mutable.
* The one known requirement is corejs mutating the Array prototype.
*/
function hardenPrototypesPostPolyfill() {
// @see https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Object/seal
// > The Object.seal() static method seals an object.
// > Sealing an object prevents extensions and makes existing properties non-configurable.
// > A sealed object has a fixed set of properties: new properties cannot be added, existing properties cannot be removed,
// > their enumerability and configurability cannot be changed, and its prototype cannot be re-assigned.
// > Values of existing properties can still be changed as long as they are writable.
// Object.freeze would take this one step further, and prevent the values of the properties from being changed as well.
// This is not currently feasible for Kibana, as this functionality is required for some of the libraries that we use, such as react-dom/server.
// While Object.seal() is not a silver bullet, it does provide a good balance between security and compatibility.
// The goal is to prevent a majority of prototype pollution vulnerabilities that can be exploited by an attacker.

// ** IMPORTANT **
// This is used both in the browser and in Node.js.
// For Node.js, we _additionally_ seal most prototypes in `src/setup_node_env/harden/prototype.js`.
// This results in sealing most prototypes twice on the server, with the exception of `Array.prototype`, which is only sealed here.
// The extra seal is a no-op, but it is done to ensure that the same code is run in both environments.

Object.seal(Object.prototype);
Object.seal(Number.prototype);
Object.seal(String.prototype);
Object.seal(Function.prototype);
Object.seal(Array.prototype);
}

// Use of the `KBN_UNSAFE_DISABLE_PROTOTYPE_HARDENING` environment variable is discouraged, and should only be set to facilitate testing
// specific scenarios. This should never be set in production.
if (!process.env.KBN_UNSAFE_DISABLE_PROTOTYPE_HARDENING) {
hardenPrototypesPostPolyfill();
}
3 changes: 2 additions & 1 deletion src/setup_node_env/harden/prototype.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,8 @@ function hardenPrototypes() {
Object.seal(String.prototype);
Object.seal(Function.prototype);

// corejs currently manipulates Array.prototype, so we cannot seal it.
// corejs currently manipulates Array.prototype, so we cannot seal it here.
// this is instead sealed within `src/platform/packages/shared/kbn-security-hardening/prototype.ts`
}

module.exports = hardenPrototypes;
1 change: 1 addition & 0 deletions test/common/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ export default function () {
sourceArgs: ['--no-base-path', '--env.name=development'],
serverArgs: [
`--server.port=${kbnTestConfig.getPort()}`,
`--server.prototypeHardening=true`,
'--status.allowAnonymous=true',
// We shouldn't embed credentials into the URL since Kibana requests to Elasticsearch should
// either include `kibanaServerTestUser` credentials, or credentials provided by the test
Expand Down
Loading

0 comments on commit 9ce2dd8

Please sign in to comment.