Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -188,34 +188,34 @@ describe('parseArgsParam', () => {
});

describe('value sanitization', () => {
it("omits values that aren't in the extended alphanumeric set", () => {
expect(parseArgsParam('key:a`b')).toStrictEqual({});
expect(parseArgsParam('key:a~b')).toStrictEqual({});
expect(parseArgsParam('key:a!b')).toStrictEqual({});
expect(parseArgsParam('key:a@b')).toStrictEqual({});
expect(parseArgsParam('key:a#b')).toStrictEqual({});
expect(parseArgsParam('key:a$b')).toStrictEqual({});
expect(parseArgsParam('key:a%b')).toStrictEqual({});
expect(parseArgsParam('key:a^b')).toStrictEqual({});
expect(parseArgsParam('key:a&b')).toStrictEqual({});
expect(parseArgsParam('key:a*b')).toStrictEqual({});
expect(parseArgsParam('key:a(b')).toStrictEqual({});
expect(parseArgsParam('key:a)b')).toStrictEqual({});
expect(parseArgsParam('key:a=b')).toStrictEqual({});
expect(parseArgsParam('key:a[b')).toStrictEqual({});
expect(parseArgsParam('key:a]b')).toStrictEqual({});
expect(parseArgsParam('key:a{b')).toStrictEqual({});
expect(parseArgsParam('key:a}b')).toStrictEqual({});
expect(parseArgsParam('key:a\\b')).toStrictEqual({});
expect(parseArgsParam('key:a|b')).toStrictEqual({});
expect(parseArgsParam("key:a'b")).toStrictEqual({});
expect(parseArgsParam('key:a"b')).toStrictEqual({});
expect(parseArgsParam('key:a,b')).toStrictEqual({});
expect(parseArgsParam('key:a.b')).toStrictEqual({});
it('omits values containing structural delimiters', () => {
expect(parseArgsParam('key:a<b')).toStrictEqual({});
expect(parseArgsParam('key:a>b')).toStrictEqual({});
expect(parseArgsParam('key:a/b')).toStrictEqual({});
expect(parseArgsParam('key:a?b')).toStrictEqual({});
expect(parseArgsParam('key:a"b')).toStrictEqual({});
expect(parseArgsParam('key:a`b')).toStrictEqual({});
});

it('allows values containing common special characters', () => {
expect(parseArgsParam('key:a~b')).toStrictEqual({ key: 'a~b' });
expect(parseArgsParam('key:a!b')).toStrictEqual({ key: 'a!b' });
expect(parseArgsParam('key:a@b')).toStrictEqual({ key: 'a@b' });
expect(parseArgsParam('key:a#b')).toStrictEqual({ key: 'a#b' });
expect(parseArgsParam('key:a$b')).toStrictEqual({ key: 'a$b' });
expect(parseArgsParam('key:a%b')).toStrictEqual({ key: 'a%b' });
expect(parseArgsParam('key:a^b')).toStrictEqual({ key: 'a^b' });
expect(parseArgsParam('key:a&b')).toStrictEqual({ key: 'a&b' });
expect(parseArgsParam('key:a*b')).toStrictEqual({ key: 'a*b' });
expect(parseArgsParam('key:a(b')).toStrictEqual({ key: 'a(b' });
expect(parseArgsParam('key:a)b')).toStrictEqual({ key: 'a)b' });
expect(parseArgsParam('key:a{b')).toStrictEqual({ key: 'a{b' });
expect(parseArgsParam('key:a}b')).toStrictEqual({ key: 'a}b' });
expect(parseArgsParam('key:a\\b')).toStrictEqual({ key: 'a\\b' });
expect(parseArgsParam('key:a|b')).toStrictEqual({ key: 'a|b' });
expect(parseArgsParam("key:a'b")).toStrictEqual({ key: "a'b" });
expect(parseArgsParam('key:a,b')).toStrictEqual({ key: 'a,b' });
expect(parseArgsParam('key:a.b')).toStrictEqual({ key: 'a.b' });
expect(parseArgsParam('key:a/b')).toStrictEqual({ key: 'a/b' });
expect(parseArgsParam('key:a?b')).toStrictEqual({ key: 'a?b' });
});

it('allows values that are in the extended alphanumeric set', () => {
Expand All @@ -230,23 +230,23 @@ describe('parseArgsParam', () => {
expect(parseArgsParam('key:1')).toStrictEqual({ key: 1 });
expect(parseArgsParam('key:1.2')).toStrictEqual({ key: 1.2 });
expect(parseArgsParam('key:-1.2')).toStrictEqual({ key: -1.2 });
expect(parseArgsParam('key:1.')).toStrictEqual({});
expect(parseArgsParam('key:.2')).toStrictEqual({});
expect(parseArgsParam('key:1.2.3')).toStrictEqual({});
expect(parseArgsParam('key:1.')).toStrictEqual({ key: '1.' });
expect(parseArgsParam('key:.2')).toStrictEqual({ key: '.2' });
expect(parseArgsParam('key:1.2.3')).toStrictEqual({ key: '1.2.3' });
});

it('also applies to nested object and array values', () => {
expect(parseArgsParam('obj.key:a!b')).toStrictEqual({});
expect(parseArgsParam('arr[0]:a!b')).toStrictEqual({});
expect(parseArgsParam('obj.key:a<b')).toStrictEqual({});
expect(parseArgsParam('arr[0]:a<b')).toStrictEqual({});
});

it('completely omits an arg when a (deeply) nested value is invalid', () => {
expect(parseArgsParam('obj.key:a!b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[]:a!b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[0]:val;obj.arr[1]:a!b;obj.foo:val')).toStrictEqual({});
expect(parseArgsParam('arr[]:val;arr[]:a!b;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[1]:a!1;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[2]:a!1;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('obj.key:a<b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[]:a<b;obj.foo:val;obj.bar.baz:val')).toStrictEqual({});
expect(parseArgsParam('obj.arr[0]:val;obj.arr[1]:a<b;obj.foo:val')).toStrictEqual({});
expect(parseArgsParam('arr[]:val;arr[]:a<b;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[1]:a<1;key:val')).toStrictEqual({ key: 'val' });
expect(parseArgsParam('arr[0]:val;arr[2]:a<1;key:val')).toStrictEqual({ key: 'val' });
});
});
});
24 changes: 10 additions & 14 deletions code/core/src/preview-api/modules/preview-web/parseArgsParam.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,10 @@ import { type Options, parse } from 'picoquery';
import { dedent } from 'ts-dedent';

// Keep this in sync with validateArgs in router/src/utils.ts
const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/;
const KEY_REGEXP = /^[a-zA-Z0-9 _-]*$/;
// Block structural delimiters (;:), HTML-injection vectors (<>"`) and control chars in values.
// Everything else is allowed so text controls can contain special characters like / * . @ etc.
const UNSAFE_VALUE_REGEXP = /[;:<>"`\x00-\x1F\x7F]/;
const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/;
const HEX_REGEXP = /^#([a-f0-9]{3,4}|[a-f0-9]{6}|[a-f0-9]{8})$/i;
const COLOR_REGEXP =
Expand All @@ -16,30 +19,23 @@ const validateArgs = (key = '', value: unknown): boolean => {
return false;
}

if (key === '' || !VALIDATION_REGEXP.test(key)) {
if (key === '' || !KEY_REGEXP.test(key)) {
return false;
}

if (value === null || value === undefined) {
return true;
} // encoded as `!null` or `!undefined` // encoded as `!null` or `!undefined`
return true; // encoded as `!null` or `!undefined`
}

// encoded as `!null` or `!undefined`
if (value instanceof Date) {
return true;
} // encoded as modified ISO string // encoded as modified ISO string
return true; // encoded as modified ISO string
}

// encoded as modified ISO string
if (typeof value === 'number' || typeof value === 'boolean') {
return true;
}
if (typeof value === 'string') {
return (
VALIDATION_REGEXP.test(value) ||
NUMBER_REGEXP.test(value) ||
HEX_REGEXP.test(value) ||
COLOR_REGEXP.test(value)
);
return !UNSAFE_VALUE_REGEXP.test(value);
}

if (Array.isArray(value)) {
Expand Down
48 changes: 48 additions & 0 deletions code/core/src/router/utils.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -245,4 +245,52 @@ describe('buildArgsParam', () => {
expect(param).toEqual('obj.nested[1].three:3');
});
});

describe('special characters in values', () => {
it('preserves values containing slashes', () => {
const param = buildArgsParam({}, { path: 'foo/bar/baz' });
expect(param).toContain('path:');
expect(param).not.toEqual('');
});

it('preserves values containing dots', () => {
const param = buildArgsParam({}, { file: 'image.png' });
expect(param).toEqual('file:image.png');
});

it('preserves values containing @ symbols', () => {
const param = buildArgsParam({}, { email: 'user@example.com' });
expect(param).toContain('email:');
});

it('preserves values containing common punctuation', () => {
const param = buildArgsParam({}, { label: "Hello, world! It's great" });
expect(param).toContain('label:');
});

it('omits values containing semicolons (structural delimiter)', () => {
const param = buildArgsParam({}, { key: 'a;b' });
expect(param).toEqual('');
});

it('omits values containing colons (structural delimiter)', () => {
const param = buildArgsParam({}, { key: 'a:b' });
expect(param).toEqual('');
});

it('omits values containing angle brackets (HTML injection)', () => {
const param = buildArgsParam({}, { key: '<script>' });
expect(param).toEqual('');
});

it('omits values containing double quotes (HTML injection)', () => {
const param = buildArgsParam({}, { key: 'a"b' });
expect(param).toEqual('');
});

it('omits values containing backticks (HTML injection)', () => {
const param = buildArgsParam({}, { key: 'a`b' });
expect(param).toEqual('');
});
});
});
24 changes: 10 additions & 14 deletions code/core/src/router/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@ export const deepDiff = (value: any, update: any): any => {
};

// Keep this in sync with validateArgs in core-client/src/preview/parseArgsParam.ts
const VALIDATION_REGEXP = /^[a-zA-Z0-9 _-]*$/;
const KEY_REGEXP = /^[a-zA-Z0-9 _-]*$/;
// Block structural delimiters (;:), HTML-injection vectors (<>"`) and control chars in values.
// Everything else is allowed so text controls can contain special characters like / * . @ etc.
const UNSAFE_VALUE_REGEXP = /[;:<>"`\x00-\x1F\x7F]/;
const NUMBER_REGEXP = /^-?[0-9]+(\.[0-9]+)?$/;
const HEX_REGEXP = /^#([a-f0-9]{3,4}|[a-f0-9]{6}|[a-f0-9]{8})$/i;
const COLOR_REGEXP =
Expand All @@ -84,30 +87,23 @@ const validateArgs = (key = '', value: unknown): boolean => {
return false;
}

if (key === '' || !VALIDATION_REGEXP.test(key)) {
if (key === '' || !KEY_REGEXP.test(key)) {
return false;
}

if (value === null || value === undefined) {
return true;
} // encoded as `!null` or `!undefined` // encoded as `!null` or `!undefined`
return true; // encoded as `!null` or `!undefined`
}

// encoded as `!null` or `!undefined`
if (value instanceof Date) {
return true;
} // encoded as modified ISO string // encoded as modified ISO string
return true; // encoded as modified ISO string
}

// encoded as modified ISO string
if (typeof value === 'number' || typeof value === 'boolean') {
return true;
}
if (typeof value === 'string') {
return (
VALIDATION_REGEXP.test(value) ||
NUMBER_REGEXP.test(value) ||
HEX_REGEXP.test(value) ||
COLOR_REGEXP.test(value)
);
return !UNSAFE_VALUE_REGEXP.test(value);
}
if (Array.isArray(value)) {
return value.every((v) => validateArgs(key, v));
Expand Down