From 99aad034ba013642be6f582c5f4ffa0b062fa576 Mon Sep 17 00:00:00 2001 From: PaulGMardling Date: Wed, 9 Sep 2026 16:21:44 +0200 Subject: [PATCH] fix(react-charts): harden Vega-Lite expression evaluation Restrict expression calls to approved built-in identities and preserve own data fields safely through fold and aggregate transforms. Add evaluator, adapter, client-rendering and server-rendering regressions. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> --- ...-6e01aa24-5182-44a6-a575-8b2caa5d76b6.json | 6 ++ .../VegaDeclarativeChart.test.tsx | 27 ++++++ .../VegaLiteExpressionEvaluator.test.ts | 32 +++++++ .../VegaLiteExpressionEvaluator.ts | 5 +- .../VegaLiteSchemaAdapter.ts | 7 +- .../VegaLiteSchemaAdapterUT.test.tsx | 83 +++++++++++++++++++ 6 files changed, 156 insertions(+), 4 deletions(-) create mode 100644 change/@fluentui-react-charts-6e01aa24-5182-44a6-a575-8b2caa5d76b6.json diff --git a/change/@fluentui-react-charts-6e01aa24-5182-44a6-a575-8b2caa5d76b6.json b/change/@fluentui-react-charts-6e01aa24-5182-44a6-a575-8b2caa5d76b6.json new file mode 100644 index 0000000000000..ebfda5a661fd5 --- /dev/null +++ b/change/@fluentui-react-charts-6e01aa24-5182-44a6-a575-8b2caa5d76b6.json @@ -0,0 +1,6 @@ +{ + "type": "patch", + "comment": "fix(react-charts): enforce safe Vega-Lite expression calls and transform property handling", + "packageName": "@fluentui/react-charts", + "email": "paulmardling@microsoft.com" +} diff --git a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaDeclarativeChart.test.tsx b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaDeclarativeChart.test.tsx index 8f2e200812b61..db7d84e3d2c0d 100644 --- a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaDeclarativeChart.test.tsx +++ b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaDeclarativeChart.test.tsx @@ -1,5 +1,6 @@ import * as React from 'react'; import { render } from '@testing-library/react'; +import { renderToString } from 'react-dom/server'; import { VegaDeclarativeChart } from './VegaDeclarativeChart'; import type { VegaDeclarativeChartProps, VegaLiteSpec } from './VegaDeclarativeChart'; import { resetIdsForTests } from '@fluentui/react-utilities'; @@ -1246,6 +1247,32 @@ describe('VegaDeclarativeChart - More Heatmap Charts', () => { }); describe('VegaDeclarativeChart - Security', () => { + it.each(['client', 'server'])('blocks a JSON-only cross-transform evaluator bypass during %s rendering', mode => { + const spec: VegaLiteSpec = JSON.parse( + JSON.stringify({ + mark: 'line', + data: { values: [{ x: 1 }] }, + transform: [ + { fold: ['constructor'], as: ['firstKey', 'objectConstructor'] }, + { calculate: 'datum.objectConstructor', as: '__proto__' }, + { aggregate: [{ op: 'count', as: 'x' }], groupby: ['__proto__'] }, + { fold: ['constructor'], as: ['secondKey', 'callable'] }, + { calculate: "datum.callable('return 73')()", as: 'y' }, + ], + encoding: { + x: { field: 'x', type: 'quantitative' }, + y: { field: 'y', type: 'quantitative' }, + }, + }), + ); + const chart = ; + + // Only inherited fields were requested, so no data survives the first fold. + expect(() => (mode === 'client' ? render(chart) : renderToString(chart))).toThrow( + 'VegaLiteSchemaAdapter: Empty data array for LineChart', + ); + }); + it('blocks malicious calculate expression (MSRC PoC: globalThis assignment)', () => { // Exact payload from the MSRC vulnerability report const maliciousSpec: VegaLiteSpec = { diff --git a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.test.ts b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.test.ts index 6fc6181c06b69..2bda4d1777870 100644 --- a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.test.ts +++ b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.test.ts @@ -88,6 +88,11 @@ describe('VegaLiteExpressionEvaluator', () => { expect(safeEvaluateExpression('max(datum.a, datum.b)', { a: 3, b: 7 })).toBe(7); }); + it('allows nested and parenthesized safe built-in calls', () => { + expect(safeEvaluateExpression('round(abs(datum.x))', { x: -3.7 })).toBe(4); + expect(safeEvaluateExpression('(datum.roundUp ? ceil : floor)(datum.x)', { roundUp: true, x: 3.2 })).toBe(4); + }); + it('evaluates safe constants', () => { expect(safeEvaluateExpression('PI', {})).toBe(Math.PI); expect(safeEvaluateExpression('E', {})).toBe(Math.E); @@ -202,6 +207,33 @@ describe('VegaLiteExpressionEvaluator', () => { expect(() => safeEvaluateExpression('datum.x()', { x: 42 })).toThrow(); }); + it.each(['datum.callback()', "datum['callback']()", '(datum.callback)()', '(true ? datum.callback : abs)()'])( + 'rejects a data-supplied function in %s without invoking it', + expression => { + const callback = jest.fn(() => 73); + + expect(() => safeEvaluateExpression(expression, { callback })).toThrow( + 'function calls are only allowed for built-in functions', + ); + expect(callback).not.toHaveBeenCalled(); + }, + ); + + it('rejects a data-supplied function with the same name as a safe built-in', () => { + const abs = jest.fn(() => 73); + + expect(() => safeEvaluateExpression('datum.abs(-1)', { abs })).toThrow( + 'function calls are only allowed for built-in functions', + ); + expect(abs).not.toHaveBeenCalled(); + }); + + it('rejects a constructor materialized as an own data property', () => { + expect(() => safeEvaluateExpression("datum.callable('return 73')()", { callable: Function })).toThrow( + 'function calls are only allowed for built-in functions', + ); + }); + it('rejects template literals (backticks)', () => { expect(() => safeEvaluateExpression('`injected`', {})).toThrow(); }); diff --git a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.ts b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.ts index 1998eb65dd98c..f3da9f48d9a34 100644 --- a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.ts +++ b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteExpressionEvaluator.ts @@ -65,6 +65,9 @@ const SAFE_FUNCTIONS: Record unknown> = { toBoolean: (x: unknown) => Boolean(x), }; +// Data transforms can materialize functions as own properties; ownership does not make them safe to call. +const SAFE_CALLABLES = new Set(Object.values(SAFE_FUNCTIONS)); + // --------------------------------------------------------------------------- // Whitelisted constants // --------------------------------------------------------------------------- @@ -437,7 +440,7 @@ class ExpressionParser { } } else if (this._peek().type === '(') { // Function call — only safe built-in functions are callable - if (typeof value !== 'function') { + if (typeof value !== 'function' || !SAFE_CALLABLES.has(value)) { throw new Error('Safe expression evaluator: function calls are only allowed for built-in functions'); } this._advance(); diff --git a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapter.ts b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapter.ts index 56630dedd9722..dd4a2c96af567 100644 --- a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapter.ts +++ b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapter.ts @@ -181,7 +181,7 @@ function applyFoldTransform( for (const row of data) { // Create a base row without the fields being folded - const baseRow: Record = {}; + const baseRow: Record = Object.create(null); for (const [key, value] of Object.entries(row)) { if (!foldFields.includes(key)) { baseRow[key] = value; @@ -190,7 +190,7 @@ function applyFoldTransform( // Create a new row for each folded field for (const field of foldFields) { - if (field in row) { + if (Object.prototype.hasOwnProperty.call(row, field)) { result.push({ ...baseRow, [keyField]: field, @@ -272,7 +272,8 @@ function applyTransforms( }); result = Array.from(groups.entries()).map(([key, rows]) => { - const baseRow: Record = {}; + // Grouping fields and aggregate aliases are data keys, including "__proto__". + const baseRow: Record = Object.create(null); groupby.forEach((g, i) => { baseRow[g] = rows[0][g]; }); diff --git a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapterUT.test.tsx b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapterUT.test.tsx index 4c03aef60e814..f67ab2225f9c6 100644 --- a/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapterUT.test.tsx +++ b/packages/charts/react-charts/library/src/components/VegaDeclarativeChart/VegaLiteSchemaAdapterUT.test.tsx @@ -16,6 +16,89 @@ describe('VegaLiteSchemaAdapter', () => { colorMap.clear(); }); + describe('transform property handling', () => { + function transformData(values: Array>, transform: VegaLiteSpec['transform']) { + const spec: VegaLiteSpec = { + mark: 'line', + data: { values }, + transform, + encoding: { + x: { field: 'x', type: 'quantitative' }, + y: { field: 'y', type: 'quantitative' }, + }, + }; + + return transformVegaLiteToLineChartProps(spec, { current: colorMap }, false).data.lineChartData![0].data; + } + + it.each(['constructor', 'toString', '__proto__'])('does not fold the inherited %s property', field => { + const points = transformData( + [{ x: 1, y: 10 }], + [{ fold: ['y', field] }, { calculate: 'isValid(datum.value) ? 1 : 0', as: 'y' }], + ); + + expect(points).toHaveLength(1); + expect(points[0]).toMatchObject({ x: 1, y: 1 }); + }); + + it.each(['constructor', 'toString', '__proto__'])('preserves the own %s data field when folding', field => { + const points = transformData([{ x: 1, [field]: 42 }], [{ fold: [field] }, { calculate: 'datum.value', as: 'y' }]); + + expect(points).toHaveLength(1); + expect(points[0]).toMatchObject({ x: 1, y: 42 }); + }); + + it('preserves an own __proto__ field while copying other fields during fold', () => { + const points = transformData( + [{ x: 1, y: 10, ['__proto__']: { value: 42 } }], + [{ fold: ['y'] }, { calculate: 'isValid(datum.__proto__) ? datum.__proto__.value : 0', as: 'y' }], + ); + + expect(points[0]).toMatchObject({ x: 1, y: 42 }); + }); + + it('preserves an own __proto__ grouping field during aggregation', () => { + const points = transformData( + [{ x: 1, value: { amount: 42 } }], + [ + { calculate: 'datum.value', as: '__proto__' }, + { aggregate: [{ op: 'count', as: 'count' }], groupby: ['x', '__proto__'] }, + { calculate: 'isValid(datum.__proto__) ? datum.__proto__.amount : 0', as: 'y' }, + ], + ); + + expect(points[0]).toMatchObject({ x: 1, y: 42 }); + }); + + it('preserves an aggregate output named __proto__ as an own data field', () => { + const points = transformData( + [{ x: 1 }, { x: 1 }], + [ + { aggregate: [{ op: 'count', as: '__proto__' }], groupby: ['x'] }, + { calculate: 'isValid(datum.__proto__) ? datum.__proto__ : 0', as: 'y' }, + ], + ); + + expect(points[0]).toMatchObject({ x: 1, y: 2 }); + }); + + it('preserves ordinary fold, calculate and aggregate results', () => { + const points = transformData( + [ + { x: 1, first: 2, second: 3 }, + { x: 2, first: 4, second: 5 }, + ], + [ + { fold: ['first', 'second'] }, + { calculate: 'pow(datum.value, 2)', as: 'squared' }, + { aggregate: [{ op: 'sum', field: 'squared', as: 'y' }], groupby: ['x'] }, + ], + ); + + expect(points).toEqual([expect.objectContaining({ x: 1, y: 13 }), expect.objectContaining({ x: 2, y: 41 })]); + }); + }); + describe('transformVegaLiteToLineChartProps', () => { test('Should transform basic line chart with quantitative axes', () => { const spec: VegaLiteSpec = {