[v11.3.x] Alerting: Add useReturnTo hook to safely handle returnTo parameter (#96481)

Alerting: Add useReturnTo hook to safely handle returnTo parameter (#96474) Add useReturnTo hook to safely handle returnTo parameter Co-authored-by: Konrad Lalik <konrad.lalik@grafana.com> (cherry picked from commit 54cc666aa0) Co-authored-by: Kevin Minehart <5140827+kminehart@users.noreply.github.com>
2024-11-15 10:10:37 +02:00
parent a09dcda92d
commit 8ca23ea03f
6 changed files with 111 additions and 10 deletions
@@ -29,9 +29,11 @@ export const LogMessages = {
  loadedCentralAlertStateHistory: 'loaded central alert state history',
 };

-const { logInfo, logError, logMeasurement } = createMonitoringLogger('features.alerting', { module: 'Alerting' });
+const { logInfo, logError, logMeasurement, logWarning } = createMonitoringLogger('features.alerting', {
+  module: 'Alerting',
+});

-export { logError, logInfo, logMeasurement };
+export { logError, logInfo, logMeasurement, logWarning };

 // eslint-disable-next-line @typescript-eslint/no-explicit-any
 export function withPerformanceLogging<TFunc extends (...args: any[]) => Promise<any>>(
@@ -33,6 +33,7 @@ import {
 import { shouldUsePrometheusRulesPrimary } from '../../../featureToggles';
 import { useDeleteRuleFromGroup } from '../../../hooks/ruleGroup/useDeleteRuleFromGroup';
 import { useAddRuleToRuleGroup, useUpdateRuleInRuleGroup } from '../../../hooks/ruleGroup/useUpsertRuleFromRuleGroup';
+import { useReturnTo } from '../../../hooks/useReturnTo';
 import { useURLSearchParams } from '../../../hooks/useURLSearchParams';
 import { RuleFormType, RuleFormValues } from '../../../types/rule-form';
 import {
@@ -79,6 +80,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
  const [addRuleToRuleGroup] = useAddRuleToRuleGroup();
  const [updateRuleInRuleGroup] = useUpdateRuleInRuleGroup();

+  const { returnTo } = useReturnTo();
  const routeParams = useParams<{ type: string; id: string }>();
  const ruleType = translateRouteParamToRuleType(routeParams.type);

@@ -170,9 +172,9 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {

      const { dataSourceName, namespaceName, groupName } = ruleGroupIdentifier;
      if (exitOnSave) {
-        const returnTo = queryParams.get('returnTo') || getReturnToUrl(ruleGroupIdentifier, ruleDefinition);
+        const returnToUrl = returnTo || getReturnToUrl(ruleGroupIdentifier, ruleDefinition);

-        locationService.push(returnTo);
+        locationService.push(returnToUrl);
        return;
      }

@@ -190,8 +192,8 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
      existing,
      grafanaTypeRule,
      notifyApp,
-      queryParams,
      updateRuleInRuleGroup,
+      returnTo,
    ]
  );

@@ -6,7 +6,6 @@ import { config } from '@grafana/runtime';
 import { Button, CustomScrollbar, LinkButton, LoadingPlaceholder, Stack } from '@grafana/ui';
 import { usePageToolbar } from 'app/core/components/Page/Page';
 import { useAppNotification } from 'app/core/copy/appNotification';
-import { useQueryParams } from 'app/core/hooks/useQueryParams';

 import { AppChromeUpdate } from '../../../../../../core/components/AppChrome/AppChromeUpdate';
 import {
@@ -17,6 +16,7 @@ import {
 import { alertRuleApi } 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 { DEFAULT_GROUP_EVALUATION_INTERVAL, formValuesToRulerGrafanaRuleDTO } from '../../../utils/rule-form';
@@ -41,11 +41,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<RuleFormValues | undefined>(undefined);

@@ -16,6 +16,7 @@ import { PromAlertingRuleState, PromRuleType } from 'app/types/unified-alerting-
 import { defaultPageNav } from '../../RuleViewer';
 import { shouldUsePrometheusRulesPrimary } from '../../featureToggles';
 import { usePrometheusCreationConsistencyCheck } from '../../hooks/usePrometheusConsistencyCheck';
+import { useReturnTo } from '../../hooks/useReturnTo';
 import { PluginOriginBadge } from '../../plugins/PluginOriginBadge';
 import { Annotation } from '../../utils/constants';
 import { makeDashboardLink, makePanelLink, stringifyErrorLike } from '../../utils/misc';
@@ -244,9 +245,9 @@ interface TitleProps {

 export const Title = ({ name, paused = false, state, health, ruleType, ruleOrigin }: TitleProps) => {
  const styles = useStyles2(getStyles);
-  const [queryParams] = useQueryParams();
  const isRecordingRule = ruleType === PromRuleType.Recording;
-  const returnTo = queryParams.returnTo ? String(queryParams.returnTo) : '/alerting/list';
+
+  const { returnTo } = useReturnTo('/alerting/list');

  return (
    <div className={styles.title}>
@@ -0,0 +1,48 @@
+import { MemoryRouter } from 'react-router-dom-v5-compat';
+import { renderHook } from 'test/test-utils';
+
+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 }) => (
+        <MemoryRouter initialEntries={[{ search: '?returnTo=/dashboard/db/my-dashboard' }]}>{children}</MemoryRouter>
+      ),
+    });
+
+    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 }) => (
+        <MemoryRouter initialEntries={[{ search: '?returnTo=https://example.com' }]}>{children}</MemoryRouter>
+      ),
+    });
+
+    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 }) => (
+        <MemoryRouter initialEntries={[{ search: '?returnTo=javascript:alert(1)' }]}>{children}</MemoryRouter>
+      ),
+    });
+
+    expect(result.current.returnTo).toBe('/fallback');
+  });
+});
@@ -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;
+  }
+}