XSS

Proof-of-concept exploit (pre-fix): 1) Run Grafana 12.3.x (or earlier) and create a dashboard with a variable description containing a markdown link with a javascript URL: Description: 'See [examples](javascript:alert("XSS"))' 2) Render the dashboard and click the link. 3) The browser executes alert("XSS") due to the javascript: URL being rendered as an anchor (no sanitization in older versions). Mitigation (post-fix in 12.4.0): The VariableDescriptionTooltip sanitizes and blocks unsafe URLs. The same input will render as plain text or a sanitized link, and no script will execute.

diff --git a/public/app/features/dashboard-scene/scene/VariableControls.tsx b/public/app/features/dashboard-scene/scene/VariableControls.tsx index 74622d5220132..7a293f227d6ec 100644 --- a/public/app/features/dashboard-scene/scene/VariableControls.tsx +++ b/public/app/features/dashboard-scene/scene/VariableControls.tsx @@ -18,6 +18,7 @@ import { useElementSelection, useStyles2 } from '@grafana/ui'; import { DashboardScene } from './DashboardScene'; import { AddVariableButton } from './VariableControlsAddButton'; +import { VariableDescriptionTooltip } from './VariableDescriptionTooltip'; export function VariableControls({ dashboard }: { dashboard: DashboardScene }) { const { variables } = sceneGraph.getVariables(dashboard)!.useState(); @@ -186,6 +187,14 @@ function VariableLabel({ } const labelOrName = state.label || state.name; + const controlsLayout = layout ?? 'horizontal'; + const descriptionSuffix = + state.description != null && state.description !== '' ? ( + <VariableDescriptionTooltip + description={state.description} + placement={controlsLayout === 'vertical' ? 'top' : 'bottom'} + /> + ) : undefined; return ( <ControlsLabel @@ -194,8 +203,9 @@ function VariableLabel({ onCancel={() => variable.onCancel?.()} label={labelOrName} error={state.error} - layout={layout ?? 'horizontal'} - description={state.description ?? undefined} + layout={controlsLayout} + description={undefined} + suffix={descriptionSuffix} className={className} /> ); diff --git a/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.test.tsx b/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.test.tsx new file mode 100644 index 0000000000000..85959d7ca313f --- /dev/null +++ b/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.test.tsx @@ -0,0 +1,60 @@ +import { render, screen } from '@testing-library/react'; + +import { Tooltip } from '@grafana/ui'; + +import { VariableDescriptionTooltip } from './VariableDescriptionTooltip'; + +function renderTooltipContent(content: React.ComponentProps<typeof Tooltip>['content']): React.ReactNode { + if (typeof content === 'function') { + return content({}); + } + + return content; +} + +jest.mock('@grafana/ui', () => { + const actual = jest.requireActual('@grafana/ui'); + + return { + ...actual, + Tooltip: ({ content, children, interactive, placement }: React.ComponentProps<typeof Tooltip>) => ( + <div data-testid="mock-tooltip" data-interactive={interactive ? 'true' : 'false'} data-placement={placement}> + <div data-testid="mock-tooltip-content">{renderTooltipContent(content)}</div> + {children} + </div> + ), + }; +}); + +describe('VariableDescriptionTooltip', () => { + it('renders markdown links as external links', () => { + render(<VariableDescriptionTooltip description={'Read [docs](https://grafana.com/docs).'} placement="bottom" />); + + const link = screen.getByRole('link', { name: 'docs' }); + expect(link).toHaveAttribute('href', 'https://grafana.com/docs'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); + + it('renders bare links as external links', () => { + render(<VariableDescriptionTooltip description={'Details: https://example.com/path?q=1.'} placement="bottom" />); + + const link = screen.getByRole('link', { name: 'https://example.com/path?q=1' }); + expect(link).toHaveAttribute('href', 'https://example.com/path?q=1'); + expect(link).toHaveAttribute('target', '_blank'); + expect(link).toHaveAttribute('rel', 'noopener noreferrer'); + }); + + it('does not render unsafe links', () => { + render(<VariableDescriptionTooltip description={'Do not click [this](javascript:alert(1))'} placement="bottom" />); + + expect(screen.queryByRole('link', { name: 'this' })).not.toBeInTheDocument(); + expect(screen.getByText(/this/)).toBeInTheDocument(); + }); + + it('uses an interactive tooltip', () => { + render(<VariableDescriptionTooltip description={'Read [docs](https://grafana.com/docs).'} placement="top" />); + + expect(screen.getByTestId('mock-tooltip')).toHaveAttribute('data-interactive', 'true'); + }); +}); diff --git a/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.tsx b/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.tsx new file mode 100644 index 0000000000000..c577521f9cbbc --- /dev/null +++ b/public/app/features/dashboard-scene/scene/VariableDescriptionTooltip.tsx @@ -0,0 +1,145 @@ +import { css } from '@emotion/css'; + +import { GrafanaTheme2 } from '@grafana/data'; +import { sanitizeUrl } from '@grafana/data/internal'; +import { t } from '@grafana/i18n'; +import { Icon, Tooltip, useStyles2 } from '@grafana/ui'; + +const MARKDOWN_LINK_REGEX = /\[([^\]]+)\]\(([^)\s]+)\)/g; +const BARE_LINK_REGEX = /https?:\/\/[^\s<>()]+/gi; +const TRAILING_PUNCTUATION_REGEX = /[),.;!?]+$/; +const SAFE_PROTOCOL_REGEX = /^https?:\/\//i; + +interface VariableDescriptionTooltipProps { + description: string; + placement: 'top' | 'bottom'; +} + +export function VariableDescriptionTooltip({ description, placement }: VariableDescriptionTooltipProps) { + const styles = useStyles2(getStyles); + + return ( + <Tooltip + content={<div className={styles.tooltipContent}>{renderDescriptionWithLinks(description, styles.link)}</div>} + placement={placement} + interactive + > + <Icon + name="info-circle" + size="sm" + className={styles.icon} + aria-label={t('dashboard.variable.description-tooltip', 'Variable description')} + /> + </Tooltip> + ); +} + +function renderDescriptionWithLinks(description: string, linkClassName: string) { + const elements: Array<string | JSX.Element> = []; + let nextKey = 0; + let cursor = 0; + + MARKDOWN_LINK_REGEX.lastIndex = 0; + + for (const match of description.matchAll(MARKDOWN_LINK_REGEX)) { + const matchStart = match.index ?? 0; + + if (matchStart > cursor) { + appendTextWithBareLinks(elements, description.slice(cursor, matchStart), linkClassName, () => nextKey++); + } + + const label = match[1]; + const url = match[2]; + elements.push(renderExternalLinkOrText(label, url, linkClassName, nextKey++)); + cursor = matchStart + match[0].length; + } + + if (cursor < description.length) { + appendTextWithBareLinks(elements, description.slice(cursor), linkClassName, () => nextKey++); + } + + return elements; +} + +function appendTextWithBareLinks( + elements: Array<string | JSX.Element>, + text: string, + linkClassName: string, + getNextKey: () => number +) { + BARE_LINK_REGEX.lastIndex = 0; + let cursor = 0; + + for (const match of text.matchAll(BARE_LINK_REGEX)) { + const matchStart = match.index ?? 0; + if (matchStart > cursor) { + elements.push(text.slice(cursor, matchStart)); + } + + const rawMatch = match[0]; + const trimmedUrl = rawMatch.replace(TRAILING_PUNCTUATION_REGEX, ''); + const trailingText = rawMatch.slice(trimmedUrl.length); + + elements.push(renderExternalLinkOrText(trimmedUrl, trimmedUrl, linkClassName, getNextKey())); + + if (trailingText) { + elements.push(trailingText); + } + + cursor = matchStart + rawMatch.length; + } + + if (cursor < text.length) { + elements.push(text.slice(cursor)); + } +} + +function renderExternalLinkOrText( + label: string, + url: string, + linkClassName: string, + key: number +): string | JSX.Element { + const safeUrl = getSafeExternalUrl(url); + + if (!safeUrl) { + return label; + } + + return ( + <a key={key} href={safeUrl} target="_blank" rel="noopener noreferrer" className={linkClassName}> + {label} + </a> + ); +} + +function getSafeExternalUrl(url: string): string | undefined { + const trimmedUrl = url.trim(); + + if (!SAFE_PROTOCOL_REGEX.test(trimmedUrl)) { + return undefined; + } + + const sanitizedUrl = sanitizeUrl(trimmedUrl); + + if (sanitizedUrl === '' || sanitizedUrl === 'about:blank') { + return undefined; + } + + return sanitizedUrl; +} + +const getStyles = (theme: GrafanaTheme2) => ({ + icon: css({ + color: theme.colors.text.secondary, + }), + tooltipContent: css({ + maxWidth: theme.spacing(40), + whiteSpace: 'normal', + wordBreak: 'break-word', + }), + link: css({ + color: theme.colors.primary.text, + textDecoration: 'underline', + }), +}); diff --git a/public/app/features/dashboard-scene/utils/dashboardControls.test.ts b/public/app/features/dashboard-scene/utils/dashboardControls.test.ts index 6a2644f647038..aaa716c172f95 100644 --- a/public/app/features/dashboard-scene/utils/dashboardControls.test.ts +++ b/public/app/features/dashboard-scene/utils/dashboardControls.test.ts @@ -230,6 +230,39 @@ describe('dashboardControls', () => { expect(result.defaultLinks).toEqual([]); }); + it('should preserve default variable descriptions from datasources', async () => { + const refs: DataSourceRef[] = [{ uid: 'ds-1', type: 'prometheus' }]; + const describedVariable: QueryVariableKind = { + ...mockVariable1, + spec: { + ...mockVariable1.spec, + description: 'Read docs at https://grafana.com/docs', + }, + }; + + const mockDs = createMockDatasource({ + uid: 'ds-1', + type: 'prometheus', + getDefaultVariables: () => Promise.resolve([describedVariable]), + getDefaultLinks: undefined, + getRef: jest.fn(() => ({ uid: 'ds-1', type: 'prometheus' })), + }); + + const mockSrv = createMockDataSourceSrv({ + get: jest.fn(() => Promise.resolve(mockDs as DataSourceApi<DataQuery, DataSourceJsonData>)), + }); + + getDataSourceSrvMock.mockReturnValue(mockSrv); + + const result = await loadDefaultControlsFromDatasources(refs); + + expect(result.defaultVariables[0]).toMatchObject({ + spec: { + description: 'Read docs at https://grafana.com/docs', + }, + }); + }); + it('should collect default links from datasources', async () => { const refs: DataSourceRef[] = [{ uid: 'ds-1', type: 'prometheus' }]; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 1def12d8443c1..0dd7b64596a3d 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -6473,6 +6473,7 @@ "description": { "action": "Change variable description" }, + "description-tooltip": "Variable description", "hide": { "action": "Change variable hide option" },