From 1913b18d4660449b67fb63aaa1a6f84a354c55e7 Mon Sep 17 00:00:00 2001 From: Kevin Minehart <5140827+kminehart@users.noreply.github.com> Date: Thu, 14 Nov 2024 10:32:41 -0600 Subject: [PATCH] [v10.4.x] Alerting: Add useReturnTo hook to safely handle returnTo parameter (#96477) Add useReturnTo hook to safely handle returnTo parameter Co-authored-by: Konrad Lalik --- .../features/alerting/unified/Analytics.ts | 4 ++ .../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(+), 5 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 012bbd988e0..c87e9a34c54 100644 --- a/public/app/features/alerting/unified/Analytics.ts +++ b/public/app/features/alerting/unified/Analytics.ts @@ -31,6 +31,10 @@ export function logError(error: Error, context?: Record) { alertingLogger.logError(error, context); } +export function logWarning(message: string, context?: Record) { + alertingLogger.logWarning(message, context); +} + // eslint-disable-next-line @typescript-eslint/no-explicit-any export function withPerformanceLogging Promise>( func: TFunc, 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 e1f56127f28..11fa8b1080d 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 @@ -15,6 +15,7 @@ import { useDispatch } from 'app/types'; import { RuleWithLocation } from 'app/types/unified-alerting'; import { LogMessages, logInfo, trackNewAlerRuleFormError } 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'; @@ -57,7 +58,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(() => { @@ -195,7 +196,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; + } +}