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 00000000000000..ebfda5a661fd5f --- /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 8f2e200812b61b..db7d84e3d2c0d4 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 6fc6181c06b69d..2bda4d1777870a 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 1998eb65dd98cb..f3da9f48d9a349 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 56630dedd9722b..dd4a2c96af567a 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 4c03aef60e8145..f67ab2225f9c61 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 = {