From 14063e87062b2d97a1798f49a4facc7938e69326 Mon Sep 17 00:00:00 2001 From: Kevin Minehart <5140827+kminehart@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:33:10 -0600 Subject: [PATCH] [v11.0.x] Alerting: Add useReturnTo hook to safely handle returnTo parameter (#96478) Add useReturnTo hook to safely handle returnTo parameter Co-authored-by: Konrad Lalik --- .../features/alerting/unified/Analytics.ts | 6 ++- .../alert-rule-form/AlertRuleForm.tsx | 5 +- .../alert-rule-form/ModifyExportRuleForm.tsx | 5 +- .../unified/hooks/useReturnTo.test.tsx | 49 +++++++++++++++++++ .../alerting/unified/hooks/useReturnTo.ts | 49 +++++++++++++++++++ 5 files changed, 107 insertions(+), 7 deletions(-) create mode 100644 public/app/features/alerting/unified/hooks/useReturnTo.test.tsx create mode 100644 public/app/features/alerting/unified/hooks/useReturnTo.ts diff --git a/public/app/features/alerting/unified/Analytics.ts b/public/app/features/alerting/unified/Analytics.ts index ef58a579ec2..3ecf4570621 100644 --- a/public/app/features/alerting/unified/Analytics.ts +++ b/public/app/features/alerting/unified/Analytics.ts @@ -26,9 +26,11 @@ export const LogMessages = { unknownMessageFromError: 'unknown messageFromError', }; -const { logInfo, logError, logMeasurement } = createMonitoringLogger('features.alerting', { module: 'Alerting' }); +const { logInfo, logError, logMeasurement, logWarning } = createMonitoringLogger('features.alerting', { + module: 'Alerting', +}); -export { logInfo, logError, logMeasurement }; +export { logInfo, logError, logMeasurement, logWarning }; // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withPerformanceLogging Promise>( diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx index 645320c32e7..d07ee3e0b78 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx @@ -21,6 +21,7 @@ import { trackAlertRuleFormCancelled, trackAlertRuleFormSaved, } from '../../../Analytics'; +import { useReturnTo } from '../../../hooks/useReturnTo'; import { useUnifiedAlertingSelector } from '../../../hooks/useUnifiedAlertingSelector'; import { deleteRuleAction, saveRuleFormAction } from '../../../state/actions'; import { RuleFormType, RuleFormValues } from '../../../types/rule-form'; @@ -63,7 +64,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { const ruleType = translateRouteParamToRuleType(routeParams.type); const uidFromParams = routeParams.id; - const returnTo = !queryParams['returnTo'] ? '/alerting/list' : String(queryParams['returnTo']); + const { returnTo } = useReturnTo('/alerting/list'); const [showDeleteModal, setShowDeleteModal] = useState(false); const defaultValues: RuleFormValues = useMemo(() => { @@ -205,7 +206,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => { {submitState.loading && } Save rule and exit - + diff --git a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx index 87f44a6c2b9..c1d5344f1a3 100644 --- a/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx +++ b/public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx @@ -4,13 +4,13 @@ import { useAsync } from 'react-use'; import { Button, CustomScrollbar, LinkButton, LoadingPlaceholder, Stack } from '@grafana/ui'; import { useAppNotification } from 'app/core/copy/appNotification'; -import { useQueryParams } from 'app/core/hooks/useQueryParams'; import { AppChromeUpdate } from '../../../../../../core/components/AppChrome/AppChromeUpdate'; import { RulerRuleDTO, RulerRuleGroupDTO } from '../../../../../../types/unified-alerting-dto'; import { alertRuleApi, ModifyExportPayload } from '../../../api/alertRuleApi'; import { fetchRulerRulesGroup } from '../../../api/ruler'; import { useDataSourceFeatures } from '../../../hooks/useCombinedRule'; +import { useReturnTo } from '../../../hooks/useReturnTo'; import { RuleFormValues } from '../../../types/rule-form'; import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource'; import { formValuesToRulerGrafanaRuleDTO, MINUTE } from '../../../utils/rule-form'; @@ -35,11 +35,10 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor defaultValues: ruleForm, shouldFocusError: true, }); - const [queryParams] = useQueryParams(); const existing = Boolean(ruleForm); // always should be true const notifyApp = useAppNotification(); - const returnTo = !queryParams['returnTo'] ? '/alerting/list' : String(queryParams['returnTo']); + const { returnTo } = useReturnTo('/alerting/list'); const [exportData, setExportData] = useState(undefined); diff --git a/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx b/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx new file mode 100644 index 00000000000..3d09fafc4d1 --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useReturnTo.test.tsx @@ -0,0 +1,49 @@ +import { renderHook } from '@testing-library/react'; +import * as React from 'react'; +import { MemoryRouter } from 'react-router-dom'; + +import { useReturnTo } from './useReturnTo'; + +describe('useReturnTo', () => { + beforeAll(() => { + // @ts-expect-error + delete window.location; + window.location = { origin: 'https://play.grafana.net' } as Location; + }); + + it('should return the fallback value when `returnTo` is not present in the query string', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { wrapper: MemoryRouter }); + + expect(result.current.returnTo).toBe('/fallback'); + }); + + it('should return the sanitized `returnTo` value when it is present in the query string and is a valid URL within the Grafana app', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/dashboard/db/my-dashboard'); + }); + + it('should return the fallback value when `returnTo` is present in the query string but is not a valid URL within the Grafana app', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/fallback'); + }); + + it('should return the fallback value when `returnTo` is present in the query string but is a malicious JavaScript URL', () => { + const { result } = renderHook(() => useReturnTo('/fallback'), { + wrapper: ({ children }) => ( + {children} + ), + }); + + expect(result.current.returnTo).toBe('/fallback'); + }); +}); diff --git a/public/app/features/alerting/unified/hooks/useReturnTo.ts b/public/app/features/alerting/unified/hooks/useReturnTo.ts new file mode 100644 index 00000000000..b522bf2c8f6 --- /dev/null +++ b/public/app/features/alerting/unified/hooks/useReturnTo.ts @@ -0,0 +1,49 @@ +import { textUtil } from '@grafana/data'; +import { config } from '@grafana/runtime'; + +import { logWarning } from '../Analytics'; + +import { useURLSearchParams } from './useURLSearchParams'; + +/** + * This hook provides a safe way to obtain the `returnTo` URL from the query string parameter + * It validates the origin and protocol to ensure the URL is withing the Grafana app + */ +export function useReturnTo(fallback?: string): { returnTo: string | undefined } { + const emptyResult = { returnTo: fallback }; + + const [searchParams] = useURLSearchParams(); + const returnTo = searchParams.get('returnTo'); + + if (!returnTo) { + return emptyResult; + } + + const sanitizedReturnTo = textUtil.sanitizeUrl(returnTo); + const baseUrl = `${window.location.origin}/${config.appSubUrl}`; + + const sanitizedUrl = tryParseURL(sanitizedReturnTo, baseUrl); + + if (!sanitizedUrl) { + logWarning('Malformed returnTo parameter', { returnTo }); + return emptyResult; + } + + const { protocol, origin, pathname, search } = sanitizedUrl; + if (['http:', 'https:'].includes(protocol) === false || origin !== window.location.origin) { + logWarning('Malformed returnTo parameter', { returnTo }); + return emptyResult; + } + + return { returnTo: `${pathname}${search}` }; +} + +// Tries to mimic URL.parse method https://developer.mozilla.org/en-US/docs/Web/API/URL/parse_static +function tryParseURL(sanitizedReturnTo: string, baseUrl: string) { + try { + const url = new URL(sanitizedReturnTo, baseUrl); + return url; + } catch (error) { + return null; + } +}