Skip to content

Commit 22dfe45

Browse files
committed
add more validation to the user param input
1 parent 747bc3a commit 22dfe45

File tree

1 file changed

+27
-11
lines changed

1 file changed

+27
-11
lines changed

dotcom-rendering/src/server/lib/add-queryparams-to-abtests.ts

Lines changed: 27 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,24 +1,40 @@
11
import type { Handler } from 'express';
22
import { validateAsFEArticle } from '../../../src/model/validate';
33

4+
const SAFE_AB_VALUE = /^[a-zA-Z0-9_-]+$/;
5+
const MAX_LENGTH = 100;
6+
47
export const getABTestsFromQueryParams: Handler = async (req, res, next) => {
58
try {
69
const frontendData = validateAsFEArticle(req.body);
7-
810
const { config } = frontendData;
9-
1011
const queryParamsAb = req.query;
11-
12-
const SAFE_KEY = /^[a-zA-Z0-9_-]{1,100}$/;
13-
const SAFE_VALUE = /^[a-zA-Z0-9_-]{1,40}$/;
14-
1512
const filteredQuery: Record<string, string> = {};
13+
1614
for (const [key, value] of Object.entries(queryParamsAb)) {
17-
if (typeof value == 'string' && key.startsWith('ab-')) {
18-
const testId = key.replace(/^ab-/, '');
19-
if (SAFE_VALUE.test(value) && SAFE_KEY.test(key)) {
20-
filteredQuery[testId] = value;
21-
}
15+
// Only process AB test params
16+
if (!key.startsWith('ab-') || typeof value !== 'string') {
17+
continue;
18+
}
19+
20+
const testId = key.replace(/^ab-/, '');
21+
22+
// Validate both test ID and variant value
23+
// This prevents injection attacks and ensures safe values
24+
if (
25+
testId.length > 0 &&
26+
testId.length <= MAX_LENGTH &&
27+
value.length > 0 &&
28+
value.length <= MAX_LENGTH &&
29+
SAFE_AB_VALUE.test(testId) &&
30+
SAFE_AB_VALUE.test(value)
31+
) {
32+
filteredQuery[testId] = value;
33+
} else {
34+
// Log suspicious input for monitoring
35+
console.warn(
36+
`Rejected invalid AB test parameter: ${key}=${value}`,
37+
);
2238
}
2339
}
2440

0 commit comments

Comments
 (0)