Skip to content

Commit b10b744

Browse files
mainframevmainframev
authored
fix(charts): block unsafe urls in chart click-to-navigate handlers (microsoft#35857)
1 parent 80f3579 commit b10b744

File tree

12 files changed

+187
-8
lines changed

12 files changed

+187
-8
lines changed

‎change/@fluentui-react-charting-3a1e68bb-2e6a-4a45-b4cf-cf16323fbb26.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix(charts): block unsafe urls in chart click-to-navigate handlers",
4+
"packageName": "@fluentui/react-charting",
5+
"email": "vgenaev@gmail.com",
6+
"dependentChangeType": "patch"
7+
}

‎change/@fluentui-react-charts-202dd3bd-ebc4-49dc-9d84-f7a781e9fa6d.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "fix(charts): block unsafe urls in chart click-to-navigate handlers",
4+
"packageName": "@fluentui/react-charts",
5+
"email": "vgenaev@gmail.com",
6+
"dependentChangeType": "patch"
7+
}

‎packages/charts/react-charting/src/components/DonutChart/Arc/Arc.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -5,7 +5,7 @@ import { getStyles } from './Arc.styles';
55
import { IChartDataPoint } from '../index';
66
import { IArcProps, IArcStyleProps, IArcStyles } from './index';
77
import { format as d3Format } from 'd3-format';
8-
import { formatScientificLimitWidth } from '../../../utilities/index';
8+
import { formatScientificLimitWidth, isSafeUrl } from '../../../utilities/index';
99
import type { JSXElement } from '@fluentui/utilities';
1010

1111
export interface IArcState {
@@ -118,7 +118,9 @@ export class Arc extends React.Component<IArcProps, IArcState> {
118118
};
119119

120120
private _redirectToUrl(href: string | undefined): void {
121-
href ? (window.location.href = href) : '';
121+
if (href && isSafeUrl(href)) {
122+
window.location.href = href;
123+
}
122124
}
123125

124126
private _getAriaLabel = (): string => {

‎packages/charts/react-charting/src/components/GroupedVerticalBarChart/GroupedVerticalBarChart.base.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@ import {
3838
calcTotalBandUnits,
3939
calcRequiredWidth,
4040
getColorFromToken,
41+
isSafeUrl,
4142
} from '../../utilities/index';
4243
import {
4344
IAccessibilityProps,
@@ -420,7 +421,9 @@ export class GroupedVerticalBarChartBase
420421
};
421422

422423
private _redirectToUrl = (href: string | undefined): void => {
423-
href ? (window.location.href = href) : '';
424+
if (href && isSafeUrl(href)) {
425+
window.location.href = href;
426+
}
424427
};
425428

426429
private _buildGraph = (

‎packages/charts/react-charting/src/components/StackedBarChart/MultiStackedBarChart.base.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@ import {
1919
formatScientificLimitWidth,
2020
getAccessibleDataObject,
2121
getNextGradient,
22+
isSafeUrl,
2223
} from '../../utilities/index';
2324
import { FocusableTooltipText } from '../../utilities/FocusableTooltipText';
2425
import type { JSXElement } from '@fluentui/utilities';
@@ -629,7 +630,9 @@ export class MultiStackedBarChartBase extends React.Component<IMultiStackedBarCh
629630
};
630631

631632
private _redirectToUrl(href: string | undefined): void {
632-
href ? (window.location.href = href) : '';
633+
if (href && isSafeUrl(href)) {
634+
window.location.href = href;
635+
}
633636
}
634637

635638
private _closeCallout = () => {

‎packages/charts/react-charting/src/components/StackedBarChart/StackedBarChart.base.tsx‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import { IAccessibilityProps, IChartDataPoint, IChartProps } from './index';
66
import { IRefArrayData, IStackedBarChartProps, IStackedBarChartStyleProps, IStackedBarChartStyles } from '../../index';
77
import { Callout, DirectionalHint } from '@fluentui/react/lib/Callout';
88
import { FocusZone, FocusZoneDirection } from '@fluentui/react-focus';
9-
import { ChartHoverCard, getAccessibleDataObject, getNextGradient } from '../../utilities/index';
9+
import { ChartHoverCard, getAccessibleDataObject, getNextGradient, isSafeUrl } from '../../utilities/index';
1010
import { FocusableTooltipText } from '../../utilities/FocusableTooltipText';
1111
import { formatToLocaleString } from '@fluentui/chart-utilities';
1212
import type { JSXElement } from '@fluentui/utilities';
@@ -508,7 +508,9 @@ export class StackedBarChartBase extends React.Component<IStackedBarChartProps,
508508
};
509509

510510
private _redirectToUrl(href: string | undefined): void {
511-
href ? (window.location.href = href) : '';
511+
if (href && isSafeUrl(href)) {
512+
window.location.href = href;
513+
}
512514
}
513515

514516
private _closeCallout = () => {

‎packages/charts/react-charting/src/components/VerticalStackedBarChart/VerticalStackedBarChart.base.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ import {
6464
calcTotalWidth,
6565
calcBandwidth,
6666
calcRequiredWidth,
67+
isSafeUrl,
6768
} from '../../utilities/index';
6869
import { IChart, IImageExportOptions } from '../../types/index';
6970
import { exportChartsAsImage } from '../../utilities/image-export-utils';
@@ -887,7 +888,9 @@ export class VerticalStackedBarChartBase
887888
mouseEvent: React.MouseEvent<SVGElement>,
888889
): void {
889890
this.props.onBarClick?.(mouseEvent, data);
890-
this.props.href ? (window.location.href = this.props.href) : '';
891+
if (this.props.href && isSafeUrl(this.props.href)) {
892+
window.location.href = this.props.href;
893+
}
891894
}
892895

893896
private _getBarGapAndScale(

‎packages/charts/react-charting/src/utilities/UtilityUnitTests.test.ts‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1513,3 +1513,70 @@ describe('generateMonthlyTicks', () => {
15131513
expect(ticks.map(d => formatLocalDate(d))).toEqual(['2020-01-01', '2020-03-01', '2020-05-01', '2020-07-01']);
15141514
});
15151515
});
1516+
1517+
describe('isSafeUrl', () => {
1518+
test('Should allow http URL', () => {
1519+
expect(utils.isSafeUrl('http://example.com')).toBe(true);
1520+
});
1521+
1522+
test('Should allow https URL', () => {
1523+
expect(utils.isSafeUrl('https://example.com')).toBe(true);
1524+
});
1525+
1526+
test('Should allow https URL with path, query, and fragment', () => {
1527+
expect(utils.isSafeUrl('https://example.com/path?q=1#section')).toBe(true);
1528+
});
1529+
1530+
test('Should allow relative path', () => {
1531+
expect(utils.isSafeUrl('/dashboard')).toBe(true);
1532+
});
1533+
1534+
test('Should allow relative path without leading slash', () => {
1535+
expect(utils.isSafeUrl('dashboard/overview')).toBe(true);
1536+
});
1537+
1538+
test('Should allow fragment-only URL', () => {
1539+
expect(utils.isSafeUrl('#section')).toBe(true);
1540+
});
1541+
1542+
test('Should allow query-only URL', () => {
1543+
expect(utils.isSafeUrl('?page=2')).toBe(true);
1544+
});
1545+
1546+
test('Should allow empty string', () => {
1547+
expect(utils.isSafeUrl('')).toBe(true);
1548+
});
1549+
1550+
test('Should block javascript: protocol', () => {
1551+
// eslint-disable-next-line no-script-url
1552+
expect(utils.isSafeUrl('javascript:alert(1)')).toBe(false);
1553+
});
1554+
1555+
test('Should block data: protocol', () => {
1556+
expect(utils.isSafeUrl('data:text/html,<script>alert(1)</script>')).toBe(false);
1557+
});
1558+
1559+
test('Should block vbscript: protocol', () => {
1560+
expect(utils.isSafeUrl('vbscript:msgbox("xss")')).toBe(false);
1561+
});
1562+
1563+
test('Should block file: protocol', () => {
1564+
expect(utils.isSafeUrl('file:///etc/passwd')).toBe(false);
1565+
});
1566+
1567+
test('Should block ftp: protocol', () => {
1568+
expect(utils.isSafeUrl('ftp://example.com/file')).toBe(false);
1569+
});
1570+
1571+
test('Should block custom: protocol', () => {
1572+
expect(utils.isSafeUrl('custom:payload')).toBe(false);
1573+
});
1574+
1575+
test('Should allow a path that contains a colon but is not a scheme', () => {
1576+
expect(utils.isSafeUrl('/path/to:resource')).toBe(true);
1577+
});
1578+
1579+
test('Should allow a path starting with a digit before colon', () => {
1580+
expect(utils.isSafeUrl('123:not-a-scheme')).toBe(true);
1581+
});
1582+
});

‎packages/charts/react-charting/src/utilities/utilities.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2554,3 +2554,11 @@ const truncateTextToFitWidth = (text: string, maxWidth: number, measure: (s: str
25542554

25552555
return text.slice(0, lo) + '...';
25562556
};
2557+
2558+
export function isSafeUrl(href: string): boolean {
2559+
if (/^[a-z][a-z0-9+.-]*:/i.test(href)) {
2560+
return /^https?:/i.test(href);
2561+
}
2562+
2563+
return true;
2564+
}

‎packages/charts/react-charts/library/src/components/VerticalStackedBarChart/VerticalStackedBarChart.tsx‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -59,6 +59,7 @@ import {
5959
calcBandwidth,
6060
calcRequiredWidth,
6161
sortAxisCategories,
62+
isSafeUrl,
6263
} from '../../utilities/index';
6364
import { formatDateToLocaleString, isInvalidValue } from '@fluentui/chart-utilities';
6465
import { useImageExport } from '../../utilities/hooks';
@@ -309,7 +310,9 @@ export const VerticalStackedBarChart: React.FunctionComponent<VerticalStackedBar
309310
mouseEvent: React.MouseEvent<SVGElement>,
310311
): void => {
311312
props.onBarClick?.(mouseEvent, data);
312-
props.href ? (window.location.href = props.href) : '';
313+
if (props.href && isSafeUrl(props.href)) {
314+
window.location.href = props.href;
315+
}
313316
};
314317

315318
function _adjustProps(): void {

0 commit comments

Comments
 (0)