Skip to content

Commit 6649ad3

Browse files
kminehartkonrad147
andauthored
[v11.2.x] Alerting: Add useReturnTo hook to safely handle returnTo parameter (grafana#96480)
Add useReturnTo hook to safely handle returnTo parameter Co-authored-by: Konrad Lalik <[email protected]>
1 parent 14043ae commit 6649ad3

File tree

6 files changed

+110
-10
lines changed

6 files changed

+110
-10
lines changed

public/app/features/alerting/unified/Analytics.ts

+4-2
Original file line numberDiff line numberDiff line change
@@ -29,9 +29,11 @@ export const LogMessages = {
2929
loadedCentralAlertStateHistory: 'loaded central alert state history',
3030
};
3131

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

34-
export { logError, logInfo, logMeasurement };
36+
export { logError, logInfo, logMeasurement, logWarning };
3537

3638
// eslint-disable-next-line @typescript-eslint/no-explicit-any
3739
export function withPerformanceLogging<TFunc extends (...args: any[]) => Promise<any>>(

public/app/features/alerting/unified/components/rule-editor/alert-rule-form/AlertRuleForm.tsx

+4-3
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
trackAlertRuleFormSaved,
3131
} from '../../../Analytics';
3232
import { useDeleteRuleFromGroup } from '../../../hooks/ruleGroup/useDeleteRuleFromGroup';
33+
import { useReturnTo } from '../../../hooks/useReturnTo';
3334
import { useUnifiedAlertingSelector } from '../../../hooks/useUnifiedAlertingSelector';
3435
import { saveRuleFormAction } from '../../../state/actions';
3536
import { RuleFormType, RuleFormValues } from '../../../types/rule-form';
@@ -72,7 +73,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
7273
const ruleType = translateRouteParamToRuleType(routeParams.type);
7374
const uidFromParams = routeParams.id;
7475

75-
const returnTo = !queryParams.returnTo ? '/alerting/list' : String(queryParams.returnTo);
76+
const { returnTo } = useReturnTo('/alerting/list');
7677
const [showDeleteModal, setShowDeleteModal] = useState<boolean>(false);
7778

7879
const defaultValues: RuleFormValues = useMemo(() => {
@@ -163,7 +164,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
163164
const ruleGroupIdentifier = getRuleGroupLocationFromRuleWithLocation(existing);
164165

165166
await deleteRuleFromGroup.execute(ruleGroupIdentifier, existing.rule);
166-
locationService.replace(returnTo);
167+
locationService.replace(returnTo ?? '/alerting/list');
167168
}
168169
};
169170

@@ -210,7 +211,7 @@ export const AlertRuleForm = ({ existing, prefill }: Props) => {
210211
{submitState.loading && <Spinner className={styles.buttonSpinner} inline={true} />}
211212
Save rule and exit
212213
</Button>
213-
<Link to={returnTo}>
214+
<Link to={returnTo ?? '/alerting/list'}>
214215
<Button variant="secondary" disabled={submitState.loading} type="button" onClick={cancelRuleCreation} size="sm">
215216
Cancel
216217
</Button>

public/app/features/alerting/unified/components/rule-editor/alert-rule-form/ModifyExportRuleForm.tsx

+2-3
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@ import { useAsync } from 'react-use';
44

55
import { Button, CustomScrollbar, LinkButton, LoadingPlaceholder, Stack } from '@grafana/ui';
66
import { useAppNotification } from 'app/core/copy/appNotification';
7-
import { useQueryParams } from 'app/core/hooks/useQueryParams';
87

98
import { AppChromeUpdate } from '../../../../../../core/components/AppChrome/AppChromeUpdate';
109
import {
@@ -15,6 +14,7 @@ import {
1514
import { alertRuleApi } from '../../../api/alertRuleApi';
1615
import { fetchRulerRulesGroup } from '../../../api/ruler';
1716
import { useDataSourceFeatures } from '../../../hooks/useCombinedRule';
17+
import { useReturnTo } from '../../../hooks/useReturnTo';
1818
import { RuleFormValues } from '../../../types/rule-form';
1919
import { GRAFANA_RULES_SOURCE_NAME } from '../../../utils/datasource';
2020
import { DEFAULT_GROUP_EVALUATION_INTERVAL, formValuesToRulerGrafanaRuleDTO } from '../../../utils/rule-form';
@@ -39,11 +39,10 @@ export function ModifyExportRuleForm({ ruleForm, alertUid }: ModifyExportRuleFor
3939
defaultValues: ruleForm,
4040
shouldFocusError: true,
4141
});
42-
const [queryParams] = useQueryParams();
4342

4443
const existing = Boolean(ruleForm); // always should be true
4544
const notifyApp = useAppNotification();
46-
const returnTo = !queryParams.returnTo ? '/alerting/list' : String(queryParams.returnTo);
45+
const { returnTo } = useReturnTo('/alerting/list');
4746

4847
const [exportData, setExportData] = useState<RuleFormValues | undefined>(undefined);
4948

public/app/features/alerting/unified/components/rule-viewer/RuleViewer.tsx

+3-2
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@ import { AlertInstanceTotalState, CombinedRule, RuleHealth, RuleIdentifier } fro
1212
import { PromAlertingRuleState, PromRuleType } from 'app/types/unified-alerting-dto';
1313

1414
import { defaultPageNav } from '../../RuleViewer';
15+
import { useReturnTo } from '../../hooks/useReturnTo';
1516
import { PluginOriginBadge } from '../../plugins/PluginOriginBadge';
1617
import { Annotation } from '../../utils/constants';
1718
import { makeDashboardLink, makePanelLink } from '../../utils/misc';
@@ -237,9 +238,9 @@ interface TitleProps {
237238

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

244245
return (
245246
<div className={styles.title}>
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,48 @@
1+
import { MemoryRouter } from 'react-router-dom';
2+
import { renderHook } from 'test/test-utils';
3+
4+
import { useReturnTo } from './useReturnTo';
5+
6+
describe('useReturnTo', () => {
7+
beforeAll(() => {
8+
// @ts-expect-error
9+
delete window.location;
10+
window.location = { origin: 'https://play.grafana.net' } as Location;
11+
});
12+
13+
it('should return the fallback value when `returnTo` is not present in the query string', () => {
14+
const { result } = renderHook(() => useReturnTo('/fallback'), { wrapper: MemoryRouter });
15+
16+
expect(result.current.returnTo).toBe('/fallback');
17+
});
18+
19+
it('should return the sanitized `returnTo` value when it is present in the query string and is a valid URL within the Grafana app', () => {
20+
const { result } = renderHook(() => useReturnTo('/fallback'), {
21+
wrapper: ({ children }) => (
22+
<MemoryRouter initialEntries={[{ search: '?returnTo=/dashboard/db/my-dashboard' }]}>{children}</MemoryRouter>
23+
),
24+
});
25+
26+
expect(result.current.returnTo).toBe('/dashboard/db/my-dashboard');
27+
});
28+
29+
it('should return the fallback value when `returnTo` is present in the query string but is not a valid URL within the Grafana app', () => {
30+
const { result } = renderHook(() => useReturnTo('/fallback'), {
31+
wrapper: ({ children }) => (
32+
<MemoryRouter initialEntries={[{ search: '?returnTo=https://example.com' }]}>{children}</MemoryRouter>
33+
),
34+
});
35+
36+
expect(result.current.returnTo).toBe('/fallback');
37+
});
38+
39+
it('should return the fallback value when `returnTo` is present in the query string but is a malicious JavaScript URL', () => {
40+
const { result } = renderHook(() => useReturnTo('/fallback'), {
41+
wrapper: ({ children }) => (
42+
<MemoryRouter initialEntries={[{ search: '?returnTo=javascript:alert(1)' }]}>{children}</MemoryRouter>
43+
),
44+
});
45+
46+
expect(result.current.returnTo).toBe('/fallback');
47+
});
48+
});
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,49 @@
1+
import { textUtil } from '@grafana/data';
2+
import { config } from '@grafana/runtime';
3+
4+
import { logWarning } from '../Analytics';
5+
6+
import { useURLSearchParams } from './useURLSearchParams';
7+
8+
/**
9+
* This hook provides a safe way to obtain the `returnTo` URL from the query string parameter
10+
* It validates the origin and protocol to ensure the URL is withing the Grafana app
11+
*/
12+
export function useReturnTo(fallback?: string): { returnTo: string | undefined } {
13+
const emptyResult = { returnTo: fallback };
14+
15+
const [searchParams] = useURLSearchParams();
16+
const returnTo = searchParams.get('returnTo');
17+
18+
if (!returnTo) {
19+
return emptyResult;
20+
}
21+
22+
const sanitizedReturnTo = textUtil.sanitizeUrl(returnTo);
23+
const baseUrl = `${window.location.origin}/${config.appSubUrl}`;
24+
25+
const sanitizedUrl = tryParseURL(sanitizedReturnTo, baseUrl);
26+
27+
if (!sanitizedUrl) {
28+
logWarning('Malformed returnTo parameter', { returnTo });
29+
return emptyResult;
30+
}
31+
32+
const { protocol, origin, pathname, search } = sanitizedUrl;
33+
if (['http:', 'https:'].includes(protocol) === false || origin !== window.location.origin) {
34+
logWarning('Malformed returnTo parameter', { returnTo });
35+
return emptyResult;
36+
}
37+
38+
return { returnTo: `${pathname}${search}` };
39+
}
40+
41+
// Tries to mimic URL.parse method https://developer.mozilla.org/en-US/docs/Web/API/URL/parse_static
42+
function tryParseURL(sanitizedReturnTo: string, baseUrl: string) {
43+
try {
44+
const url = new URL(sanitizedReturnTo, baseUrl);
45+
return url;
46+
} catch (error) {
47+
return null;
48+
}
49+
}

0 commit comments

Comments
 (0)