Skip to content

Commit cd1b022

Browse files
authored
fix(core): Use symbol for normalization checks (#20486)
This changes how we check for normalization meta data, making it harder to trick. Previously, theoretically some JSON.parsed payload that we normalize could have the fields set we check for normalization logic. With this change, we use a symbol which cannot be generated by JSON, improving on this.
1 parent 15a6a8b commit cd1b022

10 files changed

Lines changed: 91 additions & 34 deletions

File tree

.size-limit.js

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ module.exports = [
5656
path: 'packages/browser/build/npm/esm/prod/index.js',
5757
import: createImport('init', 'browserTracingIntegration', 'browserProfilingIntegration'),
5858
gzip: true,
59-
limit: '49 KB',
59+
limit: '50 KB',
6060
disablePlugins: ['@size-limit/esbuild'],
6161
},
6262
{
@@ -326,15 +326,15 @@ module.exports = [
326326
path: createCDNPath('bundle.tracing.replay.feedback.min.js'),
327327
gzip: false,
328328
brotli: false,
329-
limit: '270 KB',
329+
limit: '271 KB',
330330
disablePlugins: ['@size-limit/esbuild'],
331331
},
332332
{
333333
name: 'CDN Bundle (incl. Tracing, Replay, Feedback, Logs, Metrics) - uncompressed',
334334
path: createCDNPath('bundle.tracing.replay.feedback.logs.metrics.min.js'),
335335
gzip: false,
336336
brotli: false,
337-
limit: '273 KB',
337+
limit: '275 KB',
338338
disablePlugins: ['@size-limit/esbuild'],
339339
},
340340
// Next.js SDK (ESM)

packages/core/src/index.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -257,6 +257,7 @@ export {
257257
} from './utils/misc';
258258
export { isNodeEnv, loadModule } from './utils/node';
259259
export { normalize, normalizeToSize, normalizeUrlToBase } from './utils/normalize';
260+
export { setNormalizationDepthOverrideHint, setSkipNormalizationHint } from './utils/normalizationHints';
260261
export {
261262
addNonEnumerableProperty,
262263
convertToPlainObject,

packages/core/src/integrations/extraerrordata.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import type { IntegrationFn } from '../types-hoist/integration';
77
import { debug } from '../utils/debug-logger';
88
import { isError, isPlainObject } from '../utils/is';
99
import { normalize } from '../utils/normalize';
10-
import { addNonEnumerableProperty } from '../utils/object';
10+
import { setSkipNormalizationHint } from '../utils/normalizationHints';
1111
import { truncate } from '../utils/string';
1212

1313
const INTEGRATION_NAME = 'ExtraErrorData';
@@ -66,7 +66,7 @@ function _enhanceEventWithErrorData(
6666
if (isPlainObject(normalizedErrorData)) {
6767
// We mark the error data as "already normalized" here, because we don't want other normalization procedures to
6868
// potentially truncate the data we just already normalized, with a certain depth setting.
69-
addNonEnumerableProperty(normalizedErrorData, '__sentry_skip_normalization__', true);
69+
setSkipNormalizationHint(normalizedErrorData);
7070
contexts[exceptionName] = normalizedErrorData;
7171
}
7272

packages/core/src/trpc.ts

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ import { captureException } from './exports';
33
import { SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN, SEMANTIC_ATTRIBUTE_SENTRY_SOURCE } from './semanticAttributes';
44
import { startSpanManual } from './tracing';
55
import { normalize } from './utils/normalize';
6-
import { addNonEnumerableProperty } from './utils/object';
6+
import { setNormalizationDepthOverrideHint } from './utils/normalizationHints';
77

88
interface SentryTrpcMiddlewareOptions {
99
/** Whether to include procedure inputs in reported events. Defaults to `false`. */
@@ -53,9 +53,8 @@ export function trpcMiddleware(options: SentryTrpcMiddlewareOptions = {}) {
5353
procedure_type: type,
5454
};
5555

56-
addNonEnumerableProperty(
56+
setNormalizationDepthOverrideHint(
5757
trpcContext,
58-
'__sentry_override_normalization_depth__',
5958
1 + // 1 for context.input + the normal normalization depth
6059
(clientOptions?.normalizeDepth ?? 5), // 5 is a sane depth
6160
);
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { addNonEnumerableProperty } from './object';
2+
3+
/**
4+
* Internal symbols for normalization behavior. JSON and other structured user payloads cannot
5+
* carry these keys, so they cannot spoof SDK-only normalization hints.
6+
* We use Symbol.for to ensure that the symbols are the same across different modules/files.
7+
*/
8+
const SENTRY_SKIP_NORMALIZATION = Symbol.for('sentry.skipNormalization');
9+
const SENTRY_OVERRIDE_NORMALIZATION_DEPTH = Symbol.for('sentry.overrideNormalizationDepth');
10+
11+
/** Marks an object so `normalize` returns it unchanged (already-normalized SDK data). */
12+
export function setSkipNormalizationHint(obj: object): void {
13+
addNonEnumerableProperty(obj, SENTRY_SKIP_NORMALIZATION, true);
14+
}
15+
16+
/** Overrides remaining normalization depth from this object downward (e.g. Redux / Pinia state). */
17+
export function setNormalizationDepthOverrideHint(obj: object, depth: number): void {
18+
addNonEnumerableProperty(obj, SENTRY_OVERRIDE_NORMALIZATION_DEPTH, depth);
19+
}
20+
21+
/** @internal */
22+
export function hasSkipNormalizationHint(value: object) {
23+
return Boolean((value as Record<symbol, unknown>)[SENTRY_SKIP_NORMALIZATION]);
24+
}
25+
26+
/** @internal */
27+
export function getNormalizationDepthOverrideHint(value: object): number | undefined {
28+
const v = (value as Record<symbol, unknown>)[SENTRY_OVERRIDE_NORMALIZATION_DEPTH];
29+
return typeof v === 'number' ? v : undefined;
30+
}

packages/core/src/utils/normalize.ts

Lines changed: 7 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import type { Primitive } from '../types-hoist/misc';
22
import { isSyntheticEvent, isVueViewModel } from './is';
3+
import { getNormalizationDepthOverrideHint, hasSkipNormalizationHint } from './normalizationHints';
34
import { convertToPlainObject } from './object';
45
import { getFunctionName, getVueInternalName } from './stacktrace';
56

@@ -101,20 +102,15 @@ function visit(
101102

102103
// From here on, we can assert that `value` is either an object or an array.
103104

104-
// Do not normalize objects that we know have already been normalized. As a general rule, the
105-
// "__sentry_skip_normalization__" property should only be used sparingly and only should only be set on objects that
106-
// have already been normalized.
107-
if ((value as ObjOrArray<unknown>)['__sentry_skip_normalization__']) {
105+
// Do not normalize objects that we know have already been normalized. Hints use internal symbols
106+
// (see normalizationHints.ts) so user-controlled JSON cannot spoof them.
107+
if (hasSkipNormalizationHint(value)) {
108108
return value as ObjOrArray<unknown>;
109109
}
110110

111-
// We can set `__sentry_override_normalization_depth__` on an object to ensure that from there
112-
// We keep a certain amount of depth.
113-
// This should be used sparingly, e.g. we use it for the redux integration to ensure we get a certain amount of state.
114-
const remainingDepth =
115-
typeof (value as ObjOrArray<unknown>)['__sentry_override_normalization_depth__'] === 'number'
116-
? ((value as ObjOrArray<unknown>)['__sentry_override_normalization_depth__'] as number)
117-
: depth;
111+
// Override remaining depth from this node (e.g. Redux / Pinia state). Set via setNormalizationDepthOverrideHint.
112+
const overrideDepth = getNormalizationDepthOverrideHint(value);
113+
const remainingDepth = overrideDepth !== undefined ? overrideDepth : depth;
118114

119115
// We're also done if we've reached the max depth
120116
if (remainingDepth === 0) {

packages/core/src/utils/object.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ export function fill(source: { [key: string]: any }, name: string, replacementFa
5353
* @param name The name of the property to be set
5454
* @param value The value to which to set the property
5555
*/
56-
export function addNonEnumerableProperty(obj: object, name: string, value: unknown): void {
56+
export function addNonEnumerableProperty(obj: object, name: string | symbol, value: unknown): void {
5757
try {
5858
Object.defineProperty(obj, name, {
5959
// enumerable: false, // the default, so we can save on bundle size by not explicitly setting it
@@ -62,7 +62,7 @@ export function addNonEnumerableProperty(obj: object, name: string, value: unkno
6262
configurable: true,
6363
});
6464
} catch {
65-
DEBUG_BUILD && debug.log(`Failed to add non-enumerable property "${name}" to object`, obj);
65+
DEBUG_BUILD && debug.log(`Failed to add non-enumerable property "${String(name)}" to object`, obj);
6666
}
6767
}
6868

packages/core/test/lib/utils/normalize.test.ts

Lines changed: 28 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@
33
*/
44

55
import { describe, expect, test, vi } from 'vitest';
6-
import { addNonEnumerableProperty, normalize } from '../../../src';
6+
import { normalize, setNormalizationDepthOverrideHint, setSkipNormalizationHint } from '../../../src';
77
import * as isModule from '../../../src/utils/is';
88
import * as stacktraceModule from '../../../src/utils/stacktrace';
99

@@ -655,15 +655,36 @@ describe('normalize()', () => {
655655
});
656656
});
657657

658-
describe('skips normalizing objects marked with a non-enumerable property __sentry_skip_normalization__', () => {
658+
describe('regression: JSON cannot spoof skip-normalization via string keys', () => {
659+
test('__sentry_skip_normalization__ as an own string property is still normalized', () => {
660+
function someFun(): void {
661+
/* no-empty */
662+
}
663+
const jsonLikePayload = {
664+
__sentry_skip_normalization__: true,
665+
nan: NaN,
666+
fun: someFun,
667+
};
668+
669+
const result = normalize(jsonLikePayload);
670+
671+
expect(result).toEqual({
672+
__sentry_skip_normalization__: true,
673+
nan: '[NaN]',
674+
fun: '[Function: someFun]',
675+
});
676+
});
677+
});
678+
679+
describe('skips normalizing objects marked with setSkipNormalizationHint (internal symbol)', () => {
659680
test('by leaving non-serializable values intact', () => {
660681
const someFun = () => undefined;
661682
const alreadyNormalizedObj = {
662683
nan: NaN,
663684
fun: someFun,
664685
};
665686

666-
addNonEnumerableProperty(alreadyNormalizedObj, '__sentry_skip_normalization__', true);
687+
setSkipNormalizationHint(alreadyNormalizedObj);
667688

668689
const result = normalize(alreadyNormalizedObj);
669690
expect(result).toEqual({
@@ -681,7 +702,7 @@ describe('normalize()', () => {
681702
},
682703
};
683704

684-
addNonEnumerableProperty(alreadyNormalizedObj, '__sentry_skip_normalization__', true);
705+
setSkipNormalizationHint(alreadyNormalizedObj);
685706

686707
const obj = {
687708
foo: {
@@ -703,7 +724,7 @@ describe('normalize()', () => {
703724
});
704725
});
705726

706-
describe('overrides normalization depth with a non-enumerable property __sentry_override_normalization_depth__', () => {
727+
describe('overrides normalization depth with setNormalizationDepthOverrideHint', () => {
707728
test('by increasing depth if it is higher', () => {
708729
const normalizationTarget = {
709730
foo: 'bar',
@@ -717,7 +738,7 @@ describe('normalize()', () => {
717738
},
718739
};
719740

720-
addNonEnumerableProperty(normalizationTarget, '__sentry_override_normalization_depth__', 3);
741+
setNormalizationDepthOverrideHint(normalizationTarget, 3);
721742

722743
const result = normalize(normalizationTarget, 1);
723744

@@ -745,7 +766,7 @@ describe('normalize()', () => {
745766
},
746767
};
747768

748-
addNonEnumerableProperty(normalizationTarget, '__sentry_override_normalization_depth__', 1);
769+
setNormalizationDepthOverrideHint(normalizationTarget, 1);
749770

750771
const result = normalize(normalizationTarget, 3);
751772

packages/react/src/redux.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,12 @@
11
/* eslint-disable @typescript-eslint/no-explicit-any */
22
import type { Scope } from '@sentry/core';
3-
import { addBreadcrumb, addNonEnumerableProperty, getClient, getCurrentScope, getGlobalScope } from '@sentry/core';
3+
import {
4+
addBreadcrumb,
5+
getClient,
6+
getCurrentScope,
7+
getGlobalScope,
8+
setNormalizationDepthOverrideHint,
9+
} from '@sentry/core';
410

511
interface Action<T = any> {
612
type: T;
@@ -138,9 +144,8 @@ function createReduxEnhancer(enhancerOptions?: Partial<SentryEnhancerOptions>):
138144

139145
// Set the normalization depth of the redux state to the configured `normalizeDepth` option or a sane number as a fallback
140146
const newStateContext = { state: { type: 'redux', value: transformedState } };
141-
addNonEnumerableProperty(
147+
setNormalizationDepthOverrideHint(
142148
newStateContext,
143-
'__sentry_override_normalization_depth__',
144149
3 + // 3 layers for `state.value.transformedState`
145150
normalizationDepth, // rest for the actual state
146151
);

packages/vue/src/pinia.ts

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,10 @@
1-
import { addBreadcrumb, addNonEnumerableProperty, getClient, getCurrentScope, getGlobalScope } from '@sentry/core';
1+
import {
2+
addBreadcrumb,
3+
getClient,
4+
getCurrentScope,
5+
getGlobalScope,
6+
setNormalizationDepthOverrideHint,
7+
} from '@sentry/core';
28
import type { Ref } from 'vue';
39

410
// Inline Pinia types
@@ -112,9 +118,8 @@ export const createSentryPiniaPlugin: (
112118
state: piniaStateContext,
113119
};
114120

115-
addNonEnumerableProperty(
121+
setNormalizationDepthOverrideHint(
116122
newState,
117-
'__sentry_override_normalization_depth__',
118123
3 + // 3 layers for `state.value.transformedState
119124
normalizationDepth, // rest for the actual state
120125
);

0 commit comments

Comments
 (0)