Skip to content

Commit 81e590d

Browse files
protect: expose uploaded-file data so rules can inspect content (#115)
The engine saw an upload's filename and the raw multipart blob, but not file content as a first-class thing (file_contains was a no-op), so a polyglot — a valid image with a malicious payload inside (the ImageMagick "PNG that isn't a PNG" RCE class) — passed a filename rule. Expose the DATA, keep the detection in rules. The multipart parser now captures each file part as { filename, type, content }, and the resolver exposes: - files.<name>.content the file bytes (contains/regex signature rules) - files.<name>.type the part's declared content-type - files.<name>.filename the filename (same as bare files.<name>) Bare files.<name> still returns the filename, so existing filename rules are unchanged. No detection heuristics live in the engine — "what's malicious" (webshell signatures, declared-type-vs-content mismatch) is composed in rules (see the triage-vpatch-npm upload template): e.g. mismatch = `files.f.type` matches ^image/ AND `files.f.content` head is markup (`^\s*<`), two inclusive conditions — which also avoids the false positive of <script> in an image's EXIF/XMP metadata. Content rides inside the already-capped body. Adds tests/protect/upload-inspection.test.ts; updates multipart shape assertions. Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
1 parent e69337e commit 81e590d

5 files changed

Lines changed: 116 additions & 8 deletions

File tree

src/protect/engine/fetch.js

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -185,7 +185,12 @@ export function parseMultipart(rawBody, boundary) {
185185
const content = part.slice(sep.index + sep[0].length).replace(/\r?\n$/, '');
186186
const filename = /filename="([^"]*)"/i.exec(disposition)?.[1];
187187
if (filename !== undefined) {
188-
files[name] = name in files ? [].concat(files[name], filename) : filename;
188+
// Capture the part's declared content-type and CONTENT (not just the filename), so rules can
189+
// inspect an upload's bytes (files.<name>.content) and detect a declared-vs-actual type
190+
// mismatch (files.<name>.mismatch). The content rides inside the already-capped rawBody.
191+
const partType = /content-type:\s*([^\r\n;]+)/i.exec(rawHeaders)?.[1]?.trim() || '';
192+
const file = { filename, type: partType, content };
193+
files[name] = name in files ? [].concat(files[name], file) : file;
189194
} else {
190195
body[name] = name in body ? [].concat(body[name], content) : content;
191196
}

src/protect/engine/request.js

Lines changed: 36 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,12 @@
1+
// Resolvable DATA attributes of an uploaded file part (files.<name>.<attr>). The engine only exposes
2+
// the raw data — WHAT counts as a malicious upload (signatures, type-vs-content mismatch) is expressed
3+
// in rules (see the triage-vpatch-npm skill), not hardcoded here.
4+
const FILE_ATTRS = new Set(['content', 'filename', 'type']);
5+
6+
// A captured file part is { filename, type, content }; tolerate the legacy bare-filename string.
7+
const fileFilename = (f) => (f && typeof f === 'object' ? f.filename : f);
8+
const fileAttribute = (f, attr) => (f && typeof f === 'object' ? f[attr] : attr === 'filename' ? f : undefined);
9+
110
// WinterCG-safe base64 decode: use Buffer on Node, fall back to atob/TextDecoder on
211
// edge runtimes (Cloudflare Workers, Deno, Bun) where Buffer may be absent. Keeps the
312
// engine hot path free of Node-only APIs (per the ADR engine-language decision).
@@ -253,16 +262,38 @@ export class RequestResolver {
253262

254263
#resolveFiles(key) {
255264
const files = this.#req.files;
256-
if (!files) {
265+
if (!files || typeof files !== 'object') {
257266
return [];
258267
}
259268

260-
if (key.endsWith('*')) {
261-
return this.#resolveWildcard(files, key);
269+
// files.<name>.<attr> — content | filename | type. Fans out over multiple files uploaded under
270+
// the same field name.
271+
const dot = key.lastIndexOf('.');
272+
if (dot !== -1 && FILE_ATTRS.has(key.slice(dot + 1)) && Object.prototype.hasOwnProperty.call(files, key.slice(0, dot))) {
273+
const attr = key.slice(dot + 1);
274+
const entry = files[key.slice(0, dot)];
275+
const list = Array.isArray(entry) ? entry : [entry];
276+
const out = [];
277+
for (const f of list) {
278+
const v = fileAttribute(f, attr);
279+
if (v !== undefined && v !== '') out.push(v);
280+
}
281+
return out;
262282
}
263283

264-
const value = files[key];
265-
return value !== undefined ? [value] : [];
284+
// Bare files.<name> (or wildcard) → the filename(s), preserving the legacy behavior that
285+
// filename-scoped rules rely on (the parser now stores a { filename, type, content } object).
286+
const filenamesOf = (entry) => (Array.isArray(entry) ? entry.map(fileFilename) : [fileFilename(entry)]);
287+
if (key.endsWith('*')) {
288+
const prefix = key.slice(0, -1);
289+
const out = [];
290+
for (const [k, entry] of Object.entries(files)) {
291+
if (k.startsWith(prefix)) out.push(...filenamesOf(entry));
292+
}
293+
return out.filter((v) => v !== undefined);
294+
}
295+
if (!Object.prototype.hasOwnProperty.call(files, key)) return [];
296+
return filenamesOf(files[key]).filter((v) => v !== undefined);
266297
}
267298

268299
#resolveRaw() {

tests/protect/batch-hardening.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -86,7 +86,8 @@ describe('item 3 — multipart on the node adapter + comparator coercion', () =>
8686
`--${b}\r\nContent-Disposition: form-data; name="avatar"; filename="x.png"\r\n\r\nBINARY\r\n--${b}--\r\n`;
8787
const shaped: any = fromNodeRequest({ method: 'POST', url: '/x', headers: { 'content-type': `multipart/form-data; boundary=${b}` } } as any, body);
8888
expect(shaped.body.comment).toBe('<script>alert(1)</script>');
89-
expect(shaped.files.avatar).toBe('x.png');
89+
// File parts are captured as { filename, type, content } for content inspection.
90+
expect(shaped.files.avatar).toMatchObject({ filename: 'x.png', content: 'BINARY' });
9091
});
9192

9293
it('in_array / array_in_array coerce numeric rule values to match string request values', () => {

tests/protect/multipart.test.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,8 @@ describe('multipart/form-data parsing', () => {
4242
);
4343
expect(shaped.body.title).toBe('<script>alert(1)</script>');
4444
expect(shaped.body['__proto__[polluted]']).toBe('yes');
45-
expect(shaped.files.avatar).toBe('evil.svg');
45+
// File parts are now captured as { filename, type, content } (content inspection), not a bare filename.
46+
expect(shaped.files.avatar).toMatchObject({ filename: 'evil.svg', type: 'image/svg+xml', content: '<svg onload=alert(1)>' });
4647
expect(shaped._rawBody).toContain('__proto__');
4748
});
4849

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,70 @@
1+
import { describe, it, expect } from 'vitest';
2+
import { createProtection } from '../../src/protect/runtime.js';
3+
4+
// File-upload content inspection: the engine exposes an upload's DATA
5+
// (files.<name>.content / .type / .filename); WHAT is malicious is expressed entirely in rules.
6+
// These tests show the rule-composed patterns (content signature + declared-type-vs-content
7+
// mismatch) and that bare files.<name> still returns the filename for existing filename rules.
8+
9+
const B = '----PSXBOUNDARY';
10+
function upload(field: string, filename: string, type: string, content: string) {
11+
const body =
12+
`--${B}\r\nContent-Disposition: form-data; name="${field}"; filename="${filename}"\r\n` +
13+
`Content-Type: ${type}\r\n\r\n${content}\r\n--${B}--\r\n`;
14+
return new Request('https://app.com/upload', {
15+
method: 'POST',
16+
headers: { 'content-type': `multipart/form-data; boundary=${B}` },
17+
body,
18+
});
19+
}
20+
const mk = (rules: any[]) => createProtection({ mode: 'block', rules: { firewall: rules, whitelists: [], whitelist_keys: {} } as any });
21+
const blocks = async (p: any, req: Request) => (await p.fetch(() => new Response('ok'))(req)).status === 403;
22+
23+
describe('files.<name>.content — signature inspection (pure rule)', () => {
24+
it('matches a webshell / ImageMagick-MSL signature in the file bytes', async () => {
25+
const p = await mk([
26+
{ id: 's', rule_v2: [{ parameter: 'files.f.content', match: { type: 'regex', value: '/<\\?php|<\\?=|<(?:read|write|msl)[\\s>]/i' } }] },
27+
]);
28+
expect(await blocks(p, upload('f', 'cat.png', 'image/png', '\x89PNG\r\n<?php system($_GET[0]); ?>'))).toBe(true);
29+
expect(await blocks(p, upload('f', 'x.jpg', 'image/jpeg', '<?xml version="1.0"?><image><read filename="/etc/passwd"/></image>'))).toBe(true);
30+
expect(await blocks(p, upload('f', 'cat.png', 'image/png', '\x89PNG a normal image'))).toBe(false);
31+
});
32+
});
33+
34+
describe('type-vs-content mismatch — composed in a rule, not the engine', () => {
35+
// "declared image AND content head is markup" — two inclusive (AND) conditions on the exposed data.
36+
const mismatchRule = {
37+
id: 'm',
38+
rule_v2: [
39+
{ parameter: 'rules', rules: [
40+
{ parameter: 'files.f.type', mutations: [], match: { type: 'regex', value: '/^image\\//i' }, inclusive: true },
41+
{ parameter: 'files.f.content', match: { type: 'regex', value: '/^\\s*<[a-z!?]/i' }, inclusive: true },
42+
] },
43+
],
44+
};
45+
46+
it('flags a raster image that is really text/markup (svg-as-png, php-as-png)', async () => {
47+
const p = await mk([mismatchRule]);
48+
expect(await blocks(p, upload('f', 'cat.png', 'image/png', '<?php echo 1; ?>'))).toBe(true);
49+
expect(await blocks(p, upload('f', 'x.png', 'image/png', '<svg><script>alert(1)</script></svg>'))).toBe(true);
50+
});
51+
52+
it('does NOT flag a real image (binary head, markup only in metadata) — no false positive', async () => {
53+
const p = await mk([mismatchRule]);
54+
// Genuine binary image head; <script> only later in an EXIF/XMP-like text field.
55+
expect(await blocks(p, upload('f', 'p.jpg', 'image/jpeg', '\xff\xd8\xff\xe1 EXIF <x:xmpmeta><dc:description><script>x</script></dc:description>'))).toBe(false);
56+
expect(await blocks(p, upload('f', 'ok.png', 'image/png', '\x89PNG normal image bytes'))).toBe(false);
57+
});
58+
});
59+
60+
describe('backward compatibility', () => {
61+
it('bare files.<name> still matches the filename', async () => {
62+
const p = await mk([{ id: 'ext', rule_v2: [{ parameter: 'files.avatar', match: { type: 'contains', value: '.php' } }] }]);
63+
expect(await blocks(p, upload('avatar', 'shell.php', 'application/octet-stream', 'x'))).toBe(true);
64+
expect(await blocks(p, upload('avatar', 'ok.png', 'image/png', 'x'))).toBe(false);
65+
});
66+
it('exposes .type and .filename as sources', async () => {
67+
const p = await mk([{ id: 't', rule_v2: [{ parameter: 'files.doc.type', match: { type: 'contains', value: 'application/x-httpd-php' } }] }]);
68+
expect(await blocks(p, upload('doc', 'a.txt', 'application/x-httpd-php', 'x'))).toBe(true);
69+
});
70+
});

0 commit comments

Comments
 (0)