From b250d30da300e445c8d0bed6448499979218d83b Mon Sep 17 00:00:00 2001 From: Lahin Date: Sun, 2 Aug 2026 09:53:28 +0600 Subject: [PATCH 1/4] Tidy docs spacing in CLAUDE.md and SECURITY.md Remove unnecessary blank lines and separator markers from CLAUDE.md and SECURITY.md for cleaner, more consistent formatting. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> --- CLAUDE.md | 29 ++--------------------------- SECURITY.md | 8 -------- 2 files changed, 2 insertions(+), 35 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6311744..6064f5c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -3,18 +3,11 @@ This file gives Claude (and other AI coding assistants) the context needed to work accurately on express-audit without exploring the codebase from scratch. ---- - ## What this project is -express-audit is a static security analysis CLI for Express.js applications. It parses -source files into an AST using `@babel/parser`, runs structured rule visitors over the -tree, and reports findings with file path, line number, severity, impact, and fix. - -It does not execute application code, make network requests, or write to the project -being scanned. +express-audit is a static security analysis CLI for Express.js applications. It parses source files into an AST using `@babel/parser`, runs structured rule visitors over the tree, and reports findings with file path, line number, severity, impact, and fix. ---- +It does not execute application code, make network requests, or write to the project being scanned. ## Commands @@ -49,8 +42,6 @@ node dist/cli.js ./src `node scripts/generate-version.mjs` has not been run first. The `prebuild` script runs it automatically before `npm run build`. ---- - ## Project structure ``` @@ -98,8 +89,6 @@ examples/ secure-app/app.js Well-secured Express app ``` ---- - ## Core types ```typescript @@ -141,8 +130,6 @@ interface Finding { } ``` ---- - ## How to write a rule 1. Create `src/rules//.ts` @@ -199,8 +186,6 @@ export const myRule: Rule = { }; ``` ---- - ## Key AST helpers (`src/core/ast-helpers.ts`) | Helper | What it does | @@ -215,8 +200,6 @@ export const myRule: Rule = { | `getNodeColumn(node)` | Column number from AST node | | `getCalleeName(call)` | Returns `"obj.method"` string from a call expression | ---- - ## Key design decisions **Entry-file scoping.** Project-level rules (HTTP001, CSP001, RATE001, HEADER001, ERR002) @@ -241,8 +224,6 @@ post-discovery filter in `engine.ts`. remediation strings showing both ESM and CJS import patterns. Use it for any rule that recommends installing a package. ---- - ## How to write tests Tests use Vitest. Test helpers are in `tests/helpers.ts`. @@ -276,8 +257,6 @@ fixture name so path-based rules (e.g. `'app.js'`, `'Dockerfile'`) behave correc Every rule must have at minimum one test that fires (true positive) and one that does not (false positive check). ---- - ## Rule ID registry | Prefix | Category | Used | @@ -306,8 +285,6 @@ Every rule must have at minimum one test that fires (true positive) and one that Pick the next available number in the appropriate prefix range. ---- - ## Scoring - Each category starts at 100 points @@ -315,8 +292,6 @@ Pick the next available number in the appropriate prefix range. - Overall score is a weighted average across categories (weights in `CATEGORY_WEIGHTS`) - Score of 100 means no findings — it does not mean the app is secure ---- - ## Dependencies (runtime only) | Package | Purpose | diff --git a/SECURITY.md b/SECURITY.md index 0b111da..9bb3cbd 100644 --- a/SECURITY.md +++ b/SECURITY.md @@ -14,8 +14,6 @@ backported patches. Once the project reaches 1.0.0, this table will be updated to reflect a formal long-term support window. ---- - ## Reporting a Vulnerability **Please do not open a public GitHub issue for security vulnerabilities.** @@ -57,8 +55,6 @@ A useful report contains: If a reported issue turns out not to be a vulnerability, we will let you know promptly and explain the reasoning. ---- - ## Responsible Disclosure We ask that you: @@ -92,8 +88,6 @@ Out of scope: vulnerable for demonstration purposes - General bugs that have no security impact (open a regular issue instead) ---- - ## Vulnerability Disclosure History No vulnerabilities have been reported or disclosed to date. @@ -101,8 +95,6 @@ No vulnerabilities have been reported or disclosed to date. This section will be updated with CVE identifiers and release links as the project matures. ---- - ## Contact Maintainer: **Muhammad Lahin** From 99f5888464d4f200f74e704bf46aaebe68b8cc8b Mon Sep 17 00:00:00 2001 From: Lahin Date: Sun, 2 Aug 2026 23:58:04 +0600 Subject: [PATCH 2/4] Add PP001 prototype pollution detection rule Introduce PP001: detects prototype-pollution patterns (Object.assign, lodash/merge-like calls, and computed property assignments) when merging or using user-controlled input (req.body/query/params/headers). Adds src/rules/validation/prototype-pollution.ts, exports the rule from validation/index.ts, updates docs/rules/README.md, and adds unit tests in tests/rules/validation.test.ts. Reports high-severity findings with remediation guidance and references. --- docs/rules/README.md | 1 + src/rules/validation/index.ts | 3 + src/rules/validation/prototype-pollution.ts | 245 ++++++++++++++++++++ tests/rules/validation.test.ts | 122 ++++++++++ 4 files changed, 371 insertions(+) create mode 100644 src/rules/validation/prototype-pollution.ts create mode 100644 tests/rules/validation.test.ts diff --git a/docs/rules/README.md b/docs/rules/README.md index 3ed3414..a832528 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -24,6 +24,7 @@ Complete documentation for all express-audit rules. |------|----------|-------| | VAL001 | 📋 Medium | Unvalidated Request Body | | VAL002 | 📋 Medium | Unvalidated Query Parameters | +| PP001 | ⚠️ High | Prototype Pollution via Object Merge | ## SQL Security diff --git a/src/rules/validation/index.ts b/src/rules/validation/index.ts index 08e1db5..0152e27 100644 --- a/src/rules/validation/index.ts +++ b/src/rules/validation/index.ts @@ -1,9 +1,12 @@ export { unsafeReqBodyRule, unsafeQueryParamRule } from './input-validation.js'; +export { prototypePollutionMergeRule } from './prototype-pollution.js'; import { unsafeReqBodyRule, unsafeQueryParamRule } from './input-validation.js'; +import { prototypePollutionMergeRule } from './prototype-pollution.js'; import type { Rule } from '../../types/index.js'; export const validationRules: Rule[] = [ unsafeReqBodyRule, unsafeQueryParamRule, + prototypePollutionMergeRule, ]; diff --git a/src/rules/validation/prototype-pollution.ts b/src/rules/validation/prototype-pollution.ts new file mode 100644 index 0000000..a439328 --- /dev/null +++ b/src/rules/validation/prototype-pollution.ts @@ -0,0 +1,245 @@ +import type { Rule, RuleContext, Finding } from '../../types/index.js'; +import type { File } from '@babel/types'; +import { traverse, getNodeLine } from '../../core/ast-helpers.js'; +import type { NodePath } from '@babel/traverse'; +import type * as BabelTypes from '@babel/types'; + +/** + * Returns true if the node is a user-controlled source: + * req.body, req.query, req.params, req.headers + * Also matches deeper access like req.body.field + */ +function isUserInput(node: BabelTypes.Node): boolean { + if (node.type !== 'MemberExpression') return false; + const mem = node as BabelTypes.MemberExpression; + + // Direct: req.body / req.query / req.params / req.headers + if ( + mem.object.type === 'Identifier' && + (mem.object as BabelTypes.Identifier).name === 'req' && + mem.property.type === 'Identifier' && + ['body', 'query', 'params', 'headers'].includes( + (mem.property as BabelTypes.Identifier).name, + ) + ) return true; + + // Nested: req.body.field / req.query.name etc. + return isUserInput(mem.object); +} + +/** + * Recursively checks whether a node IS user input or CONTAINS user input + * (e.g. a template literal or binary expression that includes req.body). + */ +function containsUserInput(node: BabelTypes.Node): boolean { + if (isUserInput(node)) return true; + if (node.type === 'TemplateLiteral') { + return (node as BabelTypes.TemplateLiteral).expressions.some(containsUserInput); + } + if (node.type === 'BinaryExpression') { + const bin = node as BabelTypes.BinaryExpression; + return containsUserInput(bin.left) || containsUserInput(bin.right); + } + return false; +} + +/** + * PP001 — Prototype Pollution via Object.assign / lodash.merge with user input + * + * Detects: + * Object.assign(target, req.body) + * _.merge(target, req.body) + * lodash.merge(target, req.body) + * merge(target, req.body) — any local alias of a merge-like function + */ +export const prototypePollutionMergeRule: Rule = { + id: 'PP001', + severity: 'high', + category: 'Input Validation', + title: 'Prototype Pollution via Object Merge', + description: + 'User-controlled input is spread or merged into an object without sanitization, ' + + 'enabling prototype pollution attacks.', + detectorType: 'ast', + remediation: + 'Validate and sanitize input before merging. Use a safe merge that strips ' + + '__proto__, constructor, and prototype keys, or use structured cloning: ' + + 'JSON.parse(JSON.stringify(input)). Better yet, validate with Zod/Joi first.', + references: [ + { + title: 'OWASP – Prototype Pollution', + url: 'https://owasp.org/www-community/vulnerabilities/Prototype_Pollution', + }, + { + title: 'OWASP Top 10 2021 – A03: Injection', + url: 'https://owasp.org/Top10/A03_2021-Injection/', + }, + { + title: 'OWASP ASVS v4.0 – V5.1: Input Validation', + url: 'https://owasp.org/www-project-application-security-verification-standard/', + }, + { + title: 'CWE-1321: Improperly Controlled Modification of Object Prototype', + url: 'https://cwe.mitre.org/data/definitions/1321.html', + }, + { + title: 'Snyk – Prototype Pollution', + url: 'https://learn.snyk.io/lesson/prototype-pollution/', + }, + ], + + run(context: RuleContext): Finding[] { + if (!context.ast) return []; + + const findings: Finding[] = []; + const seen = new Set(); + const ast = context.ast as File; + + traverse(ast, { + CallExpression(path: NodePath) { + const callee = path.node.callee; + const args = path.node.arguments; + + // ── Pattern 1: Object.assign(target, userInput) ──────────────────── + const isObjectAssign = + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + (callee.object as BabelTypes.Identifier).name === 'Object' && + callee.property.type === 'Identifier' && + (callee.property as BabelTypes.Identifier).name === 'assign'; + + if (isObjectAssign && args.length >= 2) { + // Any argument after the first (target) that contains user input + const taintedArg = args.slice(1).find(a => containsUserInput(a)); + if (taintedArg) { + const line = getNodeLine(path.node); + if (!seen.has(line)) { + seen.add(line); + findings.push({ + ruleId: 'PP001', + severity: 'high', + category: 'Input Validation', + title: 'Prototype Pollution via Object.assign', + description: + 'Object.assign() called with unsanitized user input — ' + + 'an attacker can set __proto__ or constructor.prototype keys.', + impact: + 'Prototype pollution can override built-in Object properties, ' + + 'leading to application logic bypass, privilege escalation, or RCE.', + remediation: + 'Validate input first: const safe = schema.parse(req.body); ' + + 'Object.assign(target, safe). ' + + 'Or strip dangerous keys before merging.', + references: prototypePollutionMergeRule.references, + filePath: context.filePath, + line, + }); + } + } + } + + // ── Pattern 2: _.merge / lodash.merge / merge(target, userInput) ─── + const isMergeCall = (() => { + // _.merge(...) + if ( + callee.type === 'MemberExpression' && + callee.property.type === 'Identifier' && + (callee.property as BabelTypes.Identifier).name === 'merge' && + callee.object.type === 'Identifier' && + ['_', 'lodash', 'merge'].includes( + (callee.object as BabelTypes.Identifier).name, + ) + ) return true; + + // merge(...) — bare function call named merge / deepMerge / deepExtend + if ( + callee.type === 'Identifier' && + ['merge', 'deepMerge', 'deepExtend', 'extend', 'defaults'].includes( + (callee as BabelTypes.Identifier).name, + ) + ) return true; + + return false; + })(); + + if (isMergeCall && args.length >= 2) { + const taintedArg = args.slice(1).find(a => containsUserInput(a)); + if (taintedArg) { + const line = getNodeLine(path.node); + if (!seen.has(line)) { + seen.add(line); + + // Determine the function name for a clearer description + const calleeName = + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' + ? `${(callee.object as BabelTypes.Identifier).name}.merge` + : callee.type === 'Identifier' + ? (callee as BabelTypes.Identifier).name + : 'merge'; + + findings.push({ + ruleId: 'PP001', + severity: 'high', + category: 'Input Validation', + title: 'Prototype Pollution via Unsafe Merge', + description: + `${calleeName}() called with unsanitized user input — ` + + 'an attacker can inject __proto__ keys and pollute Object.prototype.', + impact: + 'Prototype pollution can override built-in Object properties, ' + + 'leading to application logic bypass, privilege escalation, or RCE.', + remediation: + 'Validate input before merging: const safe = schema.parse(req.body); ' + + `${calleeName}(target, safe). ` + + 'Use lodash@>=4.17.21 which has prototype pollution protections, ' + + 'or use structured cloning: JSON.parse(JSON.stringify(input)).', + references: prototypePollutionMergeRule.references, + filePath: context.filePath, + line, + }); + } + } + } + }, + + // ── Pattern 3: obj[req.body.key] = value (computed property assignment) ── + AssignmentExpression(path: NodePath) { + const { left } = path.node; + + if (left.type !== 'MemberExpression') return; + if (!(left as BabelTypes.MemberExpression).computed) return; + + const keyNode = (left as BabelTypes.MemberExpression).property; + if (!containsUserInput(keyNode)) return; + + const line = getNodeLine(path.node); + if (seen.has(line)) return; + seen.add(line); + + findings.push({ + ruleId: 'PP001', + severity: 'high', + category: 'Input Validation', + title: 'Prototype Pollution via Computed Property Assignment', + description: + 'Object property is set using a user-controlled key — ' + + 'an attacker can assign to __proto__ and pollute Object.prototype.', + impact: + 'Setting obj[userKey] = value where userKey is "__proto__" modifies ' + + 'the prototype chain for all objects, potentially bypassing security checks.', + remediation: + 'Validate the key before assignment: ' + + 'const SAFE_KEYS = new Set([...]); ' + + 'if (SAFE_KEYS.has(key)) obj[key] = value; ' + + 'Or use a Map instead of a plain object.', + references: prototypePollutionMergeRule.references, + filePath: context.filePath, + line, + }); + }, + }); + + return findings; + }, +}; diff --git a/tests/rules/validation.test.ts b/tests/rules/validation.test.ts new file mode 100644 index 0000000..6722b47 --- /dev/null +++ b/tests/rules/validation.test.ts @@ -0,0 +1,122 @@ +import { describe, it, expect } from 'vitest'; +import { prototypePollutionMergeRule } from '../../src/rules/validation/prototype-pollution.js'; +import { createContext } from '../helpers.js'; + +// --------------------------------------------------------------------------- +// PP001 – Object.assign with user input +// --------------------------------------------------------------------------- +describe('PP001 – Object.assign with user input', () => { + it('flags Object.assign(target, req.body)', () => { + const ctx = createContext(` + app.post('/update', (req, res) => { + Object.assign(config, req.body); + }); + `); + const findings = prototypePollutionMergeRule.run(ctx); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe('PP001'); + expect(findings[0].severity).toBe('high'); + }); + + it('flags Object.assign with req.query', () => { + const ctx = createContext(`Object.assign(defaults, req.query);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('flags Object.assign with req.params', () => { + const ctx = createContext(`Object.assign(options, req.params);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('does not flag Object.assign with no user input', () => { + const ctx = createContext(`Object.assign(target, { safe: true });`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag Object.assign with only one argument', () => { + const ctx = createContext(`Object.assign(target);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag Object.keys or Object.values', () => { + const ctx = createContext(`Object.keys(req.body);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// PP001 – lodash / merge functions with user input +// --------------------------------------------------------------------------- +describe('PP001 – lodash merge with user input', () => { + it('flags _.merge(target, req.body)', () => { + const ctx = createContext(`_.merge(config, req.body);`); + const findings = prototypePollutionMergeRule.run(ctx); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe('PP001'); + }); + + it('flags lodash.merge(target, req.body)', () => { + const ctx = createContext(`lodash.merge(defaults, req.body);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('flags bare merge(target, req.body)', () => { + const ctx = createContext(`merge(config, req.body);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('flags deepMerge(target, req.body)', () => { + const ctx = createContext(`deepMerge(defaults, req.body);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('flags extend(target, req.body)', () => { + const ctx = createContext(`extend(options, req.query);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(1); + }); + + it('does not flag _.merge with no user input', () => { + const ctx = createContext(`_.merge(defaults, { timeout: 5000 });`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag _.merge with a single safe argument', () => { + const ctx = createContext(`_.merge(a, b);`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); +}); + +// --------------------------------------------------------------------------- +// PP001 – Computed property assignment with user-controlled key +// --------------------------------------------------------------------------- +describe('PP001 – Computed property assignment', () => { + it('flags obj[req.body.key] = value', () => { + const ctx = createContext(` + app.post('/set', (req, res) => { + config[req.body.key] = req.body.value; + }); + `); + const findings = prototypePollutionMergeRule.run(ctx); + expect(findings.some(f => f.ruleId === 'PP001')).toBe(true); + }); + + it('flags obj[req.query.field] = value', () => { + const ctx = createContext(`settings[req.query.field] = req.query.value;`); + expect(prototypePollutionMergeRule.run(ctx).some(f => f.ruleId === 'PP001')).toBe(true); + }); + + it('does not flag obj[staticKey] = value', () => { + const ctx = createContext(`config['timeout'] = 5000;`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag obj.property = value (non-computed)', () => { + const ctx = createContext(`config.timeout = 5000;`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag array index assignment', () => { + const ctx = createContext(`arr[0] = 'value';`); + expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); + }); +}); From d48239c3610ca0ee36e8be8c52223be58e9169fb Mon Sep 17 00:00:00 2001 From: Lahin Date: Mon, 3 Aug 2026 00:01:25 +0600 Subject: [PATCH 3/4] Add PP001 prototype-pollution docs; update tests count Introduce documentation for a new PP001 (Prototype Pollution via Object Merge) rule: adds false-positive guidance (docs/false-positives.md) and standards mapping (docs/standards.md), and updates CLAUDE.md and README.md to include the PP category and bump the test count to 168. Documentation-only changes to reflect the new rule and test suite size. --- CLAUDE.md | 3 ++- README.md | 7 ++++--- docs/false-positives.md | 36 ++++++++++++++++++++++++++++++++++++ docs/standards.md | 14 ++++++++++++++ 4 files changed, 56 insertions(+), 4 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6064f5c..ef08b2f 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,7 +21,7 @@ node scripts/generate-version.mjs # Build TypeScript to dist/ npm run build -# Run all tests (150 tests across 15 files) +# Run all tests (168 tests across 16 files) npm test # Run tests in watch mode @@ -265,6 +265,7 @@ Every rule must have at minimum one test that fires (true positive) and one that | `AUTH` | Authentication | 001–002 | | `AUTHZ` | Authorization | 001–002 | | `VAL` | Input Validation | 001–002 | +| `PP` | Prototype Pollution | 001 | | `SQL` | SQL Security | 001–002 | | `HTTP` | HTTP Security | 001 | | `HEADER` | HTTP Headers | 001 | diff --git a/README.md b/README.md index 5b04e5c..c95156f 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ express-audit is designed to be deterministic, transparent, and privacy-friendly **Every rule maps to a published standard.** All findings reference OWASP, RFCs, CWE, or W3C specifications — not internal opinions. See [Standards & References](./docs/standards.md) for the full per-rule mapping. -**150 tests, all passing.** Every rule is tested against both a vulnerable and a secure implementation before release. See [How We Ensure Accuracy](./docs/accuracy.md) for the testing methodology and false-positive strategy. +**168 tests, all passing.** Every rule is tested against both a vulnerable and a secure implementation before release. See [How We Ensure Accuracy](./docs/accuracy.md) for the testing methodology and false-positive strategy. **Rules are documented with rationale.** Every rule in [`docs/rules/`](./docs/rules/) includes a description, a vulnerable example, a secure example, the security impact, and references to OWASP, RFCs, or official documentation — so you understand *why* a finding matters, not just *that* it fired. @@ -271,7 +271,8 @@ Hardcoded JWT secrets, missing token expiration, weak bcrypt cost factors, plain Sensitive routes (`DELETE`, `PATCH`, `PUT`) without authentication middleware, admin endpoints without role checks. ### Input Validation -Direct use of `req.body` and `req.query` without a validation library (Zod, Joi, express-validator). + +Direct use of `req.body` and `req.query` without a validation library (Zod, Joi, express-validator). Prototype pollution via `Object.assign`, `_.merge`, and computed property assignment with user-controlled input. ### SQL Security Raw query string concatenation with user input, unsafe Prisma `$queryRawUnsafe` / `$executeRawUnsafe` calls. @@ -319,7 +320,7 @@ Dozens of built-in rules across 16 categories. Full documentation for each rule |---|---|---| | `JWT`, `AUTH` | Authentication | JWT001, JWT002, AUTH001, AUTH002 | | `AUTHZ` | Authorization | AUTHZ001, AUTHZ002 | -| `VAL` | Input Validation | VAL001, VAL002 | +| `VAL`, `PP` | Input Validation | VAL001, VAL002, PP001 | | `SQL` | SQL Security | SQL001, SQL002 | | `HTTP`, `HEADER`, `CSP` | HTTP Security | HTTP001, CSP001, HEADER001 | | `COOKIE`, `SESSION` | Cookies & Sessions | COOKIE001, SESSION001 | diff --git a/docs/false-positives.md b/docs/false-positives.md index e5f6a18..e47c902 100644 --- a/docs/false-positives.md +++ b/docs/false-positives.md @@ -441,6 +441,42 @@ await myRepo.fetch(item.id); // 'fetch' is not a recognized DB method --- +### PP001 — Prototype Pollution via Object Merge + +**Might report when code is fine (false positive)** + +```js +// May fire — bare function named 'merge' that is unrelated to object merging +merge(outputStream, inputStream); // stream merge, not object merge +``` + +The rule matches any bare function call named `merge`, `deepMerge`, `extend`, or +`defaults` with a user-input argument. A stream utility or custom function that happens +to share one of those names will trigger it. + +**Fix:** rename the function or suppress the finding on that line. + +**Might stay silent when code is vulnerable (false negative)** + +```js +// ❌ Will NOT fire — user input assigned to a variable first +const data = req.body; +Object.assign(config, data); + +// ❌ Will NOT fire — custom recursive merge function not in the known list +myDeepClone(target, req.body); + +// ❌ Will NOT fire — spread into an object literal +const merged = { ...defaults, ...req.body }; +``` + +Object spread (`{ ...req.body }`) is the most common false negative. It is functionally +equivalent to `Object.assign` for prototype pollution purposes but produces a different +AST node (`ObjectExpression` with `SpreadElement`) that this rule does not currently +cover. A separate rule or an expansion of PP001 would be needed to catch it. + +--- + ## The bottom line | What the tool is good at | What it misses | diff --git a/docs/standards.md b/docs/standards.md index e3f9990..3d62c83 100644 --- a/docs/standards.md +++ b/docs/standards.md @@ -169,6 +169,20 @@ connection strings, SendGrid API keys, and generic hardcoded passwords. --- +## Input Validation + +### PP001 — Prototype Pollution via Object Merge + +| Standard | Reference | +|---|---| +| OWASP Top 10 2021 | [A03: Injection](https://owasp.org/Top10/A03_2021-Injection/) | +| OWASP ASVS v4.0 | [V5.1: Input Validation](https://owasp.org/www-project-application-security-verification-standard/) | +| OWASP | [Prototype Pollution](https://owasp.org/www-community/vulnerabilities/Prototype_Pollution) | +| CWE | [CWE-1321: Improperly Controlled Modification of Object Prototype](https://cwe.mitre.org/data/definitions/1321.html) | +| Snyk | [Prototype Pollution Guide](https://learn.snyk.io/lesson/prototype-pollution/) | + +--- + ## SQL Security ### SQL001 — SQL Injection Risk From e5a6ebb82c7d35bbe316120d24944a77200abda4 Mon Sep 17 00:00:00 2001 From: Lahin Date: Mon, 3 Aug 2026 00:17:26 +0600 Subject: [PATCH 4/4] Add INJECT001 code-injection rule and tests Introduce INJECT001: a critical Input Validation rule that detects user-controlled input passed to dynamic code sinks (eval, new Function, vm.*). Adds src/rules/validation/code-injection.ts, registers it in src/rules/validation/index.ts, and adds unit tests in tests/rules/validation.test.ts. Update docs and metadata (README.md, CLAUDE.md, docs/false-positives.md, docs/rules/README.md, docs/standards.md) to document the new rule and increase test count. Detects template literals and concatenation of req.body/query/params/headers into execution sinks. --- CLAUDE.md | 3 +- README.md | 6 +- docs/false-positives.md | 29 ++++ docs/rules/README.md | 1 + docs/standards.md | 11 ++ src/rules/validation/code-injection.ts | 185 +++++++++++++++++++++++++ src/rules/validation/index.ts | 3 + tests/rules/validation.test.ts | 88 ++++++++++++ 8 files changed, 322 insertions(+), 4 deletions(-) create mode 100644 src/rules/validation/code-injection.ts diff --git a/CLAUDE.md b/CLAUDE.md index ef08b2f..5b70aa5 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -21,7 +21,7 @@ node scripts/generate-version.mjs # Build TypeScript to dist/ npm run build -# Run all tests (168 tests across 16 files) +# Run all tests (182 tests across 16 files) npm test # Run tests in watch mode @@ -266,6 +266,7 @@ Every rule must have at minimum one test that fires (true positive) and one that | `AUTHZ` | Authorization | 001–002 | | `VAL` | Input Validation | 001–002 | | `PP` | Prototype Pollution | 001 | +| `INJECT` | Code Injection | 001 | | `SQL` | SQL Security | 001–002 | | `HTTP` | HTTP Security | 001 | | `HEADER` | HTTP Headers | 001 | diff --git a/README.md b/README.md index c95156f..44d89e6 100644 --- a/README.md +++ b/README.md @@ -34,7 +34,7 @@ express-audit is designed to be deterministic, transparent, and privacy-friendly **Every rule maps to a published standard.** All findings reference OWASP, RFCs, CWE, or W3C specifications — not internal opinions. See [Standards & References](./docs/standards.md) for the full per-rule mapping. -**168 tests, all passing.** Every rule is tested against both a vulnerable and a secure implementation before release. See [How We Ensure Accuracy](./docs/accuracy.md) for the testing methodology and false-positive strategy. +**182 tests, all passing.** Every rule is tested against both a vulnerable and a secure implementation before release. See [How We Ensure Accuracy](./docs/accuracy.md) for the testing methodology and false-positive strategy. **Rules are documented with rationale.** Every rule in [`docs/rules/`](./docs/rules/) includes a description, a vulnerable example, a secure example, the security impact, and references to OWASP, RFCs, or official documentation — so you understand *why* a finding matters, not just *that* it fired. @@ -272,7 +272,7 @@ Sensitive routes (`DELETE`, `PATCH`, `PUT`) without authentication middleware, a ### Input Validation -Direct use of `req.body` and `req.query` without a validation library (Zod, Joi, express-validator). Prototype pollution via `Object.assign`, `_.merge`, and computed property assignment with user-controlled input. +Direct use of `req.body` and `req.query` without a validation library (Zod, Joi, express-validator). Prototype pollution via `Object.assign`, `_.merge`, and computed property assignment with user-controlled input. Code injection via `eval()`, `new Function()`, and the Node.js `vm` module with user-controlled input. ### SQL Security Raw query string concatenation with user input, unsafe Prisma `$queryRawUnsafe` / `$executeRawUnsafe` calls. @@ -320,7 +320,7 @@ Dozens of built-in rules across 16 categories. Full documentation for each rule |---|---|---| | `JWT`, `AUTH` | Authentication | JWT001, JWT002, AUTH001, AUTH002 | | `AUTHZ` | Authorization | AUTHZ001, AUTHZ002 | -| `VAL`, `PP` | Input Validation | VAL001, VAL002, PP001 | +| `VAL`, `PP`, `INJECT` | Input Validation | VAL001, VAL002, PP001, INJECT001 | | `SQL` | SQL Security | SQL001, SQL002 | | `HTTP`, `HEADER`, `CSP` | HTTP Security | HTTP001, CSP001, HEADER001 | | `COOKIE`, `SESSION` | Cookies & Sessions | COOKIE001, SESSION001 | diff --git a/docs/false-positives.md b/docs/false-positives.md index e47c902..29cf896 100644 --- a/docs/false-positives.md +++ b/docs/false-positives.md @@ -477,6 +477,35 @@ cover. A separate rule or an expansion of PP001 would be needed to catch it. --- +### INJECT001 — Code Injection via eval or new Function + +**Might report when code is fine (false positive)** + +Almost none. The rule requires both a dangerous sink (`eval`, `new Function`, `vm.*`) AND traceable user input (`req.body`, `req.query`, `req.params`, `req.headers`) in the same expression. A hardcoded string passed to `eval` will not fire. + +```js +eval('1 + 1'); // will NOT fire — no user input +new Function('a', 'return a'); // will NOT fire — all string literals +``` + +**Might stay silent when code is vulnerable (false negative)** + +```js +// ❌ Will NOT fire — user input stored in a variable first +const code = req.body.script; +eval(code); + +// ❌ Will NOT fire — user input passed through a function call +eval(sanitize(req.body.code)); // even if sanitize() does nothing useful + +// ❌ Will NOT fire — indirect eval via setTimeout string form +setTimeout(req.body.code, 0); +``` + +The variable-assignment gap is the most important one. If the code assigns user input to a variable and then passes that variable to `eval`, the rule does not fire. This is a known limitation of static analysis without data-flow tracking. + +--- + ## The bottom line | What the tool is good at | What it misses | diff --git a/docs/rules/README.md b/docs/rules/README.md index a832528..f1ce2c1 100644 --- a/docs/rules/README.md +++ b/docs/rules/README.md @@ -25,6 +25,7 @@ Complete documentation for all express-audit rules. | VAL001 | 📋 Medium | Unvalidated Request Body | | VAL002 | 📋 Medium | Unvalidated Query Parameters | | PP001 | ⚠️ High | Prototype Pollution via Object Merge | +| INJECT001 | 🔴 Critical | Code Injection via eval or new Function | ## SQL Security diff --git a/docs/standards.md b/docs/standards.md index 3d62c83..b8ea6ab 100644 --- a/docs/standards.md +++ b/docs/standards.md @@ -181,6 +181,17 @@ connection strings, SendGrid API keys, and generic hardcoded passwords. | CWE | [CWE-1321: Improperly Controlled Modification of Object Prototype](https://cwe.mitre.org/data/definitions/1321.html) | | Snyk | [Prototype Pollution Guide](https://learn.snyk.io/lesson/prototype-pollution/) | +### INJECT001 — Code Injection via eval or new Function + +| Standard | Reference | +|---|---| +| OWASP Top 10 2021 | [A03: Injection](https://owasp.org/Top10/A03_2021-Injection/) | +| OWASP ASVS v4.0 | [V5.2: Sanitization and Sandboxing](https://owasp.org/www-project-application-security-verification-standard/) | +| OWASP | [Code Injection](https://owasp.org/www-community/attacks/Code_Injection) | +| CWE | [CWE-94: Improper Control of Generation of Code](https://cwe.mitre.org/data/definitions/94.html) | +| CWE | [CWE-95: Improper Neutralization of Directives in eval()](https://cwe.mitre.org/data/definitions/95.html) | +| Node.js | [vm Module Documentation](https://nodejs.org/api/vm.html) | + --- ## SQL Security diff --git a/src/rules/validation/code-injection.ts b/src/rules/validation/code-injection.ts new file mode 100644 index 0000000..99d53d0 --- /dev/null +++ b/src/rules/validation/code-injection.ts @@ -0,0 +1,185 @@ +import type { Rule, RuleContext, Finding } from '../../types/index.js'; +import type { File } from '@babel/types'; +import { traverse, getNodeLine } from '../../core/ast-helpers.js'; +import type { NodePath } from '@babel/traverse'; +import type * as BabelTypes from '@babel/types'; + +/** + * Returns true if the node is — or descends from — a user-controlled source: + * req.body, req.query, req.params, req.headers (and nested field access on those) + */ +function isUserInput(node: BabelTypes.Node): boolean { + if (node.type !== 'MemberExpression') return false; + const mem = node as BabelTypes.MemberExpression; + + // req.body / req.query / req.params / req.headers + if ( + mem.object.type === 'Identifier' && + (mem.object as BabelTypes.Identifier).name === 'req' && + mem.property.type === 'Identifier' && + ['body', 'query', 'params', 'headers'].includes( + (mem.property as BabelTypes.Identifier).name, + ) + ) return true; + + // req.body.field / req.query.name etc. + return isUserInput(mem.object); +} + +function containsUserInput(node: BabelTypes.Node): boolean { + if (isUserInput(node)) return true; + if (node.type === 'TemplateLiteral') { + return (node as BabelTypes.TemplateLiteral).expressions.some(containsUserInput); + } + if (node.type === 'BinaryExpression') { + const bin = node as BabelTypes.BinaryExpression; + return containsUserInput(bin.left) || containsUserInput(bin.right); + } + return false; +} + +/** + * INJECT001 – Code Injection via eval / new Function / vm.runInNewContext + * + * Detects user-controlled input passed to dynamic code execution sinks: + * eval(req.body.code) + * new Function(req.query.fn) + * new Function('x', req.body.expr) + * vm.runInNewContext(req.body.script) + * vm.runInThisContext(req.query.code) + * vm.Script(req.body.src) + */ +export const codeInjectionRule: Rule = { + id: 'INJECT001', + severity: 'critical', + category: 'Input Validation', + title: 'Code Injection via eval or new Function', + description: + 'User-controlled input is passed to a dynamic code execution sink ' + + '(eval, new Function, or Node.js vm module), enabling remote code execution.', + detectorType: 'ast', + remediation: + 'Never pass user input to eval(), new Function(), or vm.runInNewContext(). ' + + 'If dynamic evaluation is genuinely required, use a sandboxed interpreter ' + + 'such as isolated-vm, or evaluate the need entirely — most use cases can be ' + + 'replaced with a lookup table, JSON schema validation, or a safe expression ' + + 'parser like expr-eval.', + references: [ + { + title: 'OWASP Top 10 2021 – A03: Injection', + url: 'https://owasp.org/Top10/A03_2021-Injection/', + }, + { + title: 'OWASP ASVS v4.0 – V5.2: Sanitization and Sandboxing', + url: 'https://owasp.org/www-project-application-security-verification-standard/', + }, + { + title: 'OWASP Code Injection', + url: 'https://owasp.org/www-community/attacks/Code_Injection', + }, + { + title: 'CWE-94: Improper Control of Generation of Code (Code Injection)', + url: 'https://cwe.mitre.org/data/definitions/94.html', + }, + { + title: 'CWE-95: Improper Neutralization of Directives in eval()', + url: 'https://cwe.mitre.org/data/definitions/95.html', + }, + { + title: 'Node.js vm Module Documentation', + url: 'https://nodejs.org/api/vm.html', + }, + ], + + run(context: RuleContext): Finding[] { + if (!context.ast) return []; + + const findings: Finding[] = []; + const seen = new Set(); + const ast = context.ast as File; + + const push = (line: number, sink: string) => { + if (seen.has(line)) return; + seen.add(line); + findings.push({ + ruleId: 'INJECT001', + severity: 'critical', + category: 'Input Validation', + title: 'Code Injection via eval or new Function', + description: `User-controlled input passed to ${sink} enables arbitrary code execution`, + impact: + 'An attacker can execute arbitrary JavaScript on the server, leading to ' + + 'full server compromise, data exfiltration, or lateral movement.', + remediation: codeInjectionRule.remediation, + references: codeInjectionRule.references, + filePath: context.filePath, + line, + }); + }; + + traverse(ast, { + // ── eval(userInput) ─────────────────────────────────────────────────── + CallExpression(path: NodePath) { + const callee = path.node.callee; + const args = path.node.arguments; + + // eval(...) + if ( + callee.type === 'Identifier' && + (callee as BabelTypes.Identifier).name === 'eval' && + args.length >= 1 && + containsUserInput(args[0]) + ) { + push(getNodeLine(path.node), 'eval()'); + return; + } + + // vm.runInNewContext(code, ...) / vm.runInThisContext(code) / vm.Script(code) + if ( + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + (callee.object as BabelTypes.Identifier).name === 'vm' && + callee.property.type === 'Identifier' && + ['runInNewContext', 'runInThisContext', 'runInContext', 'compileFunction'].includes( + (callee.property as BabelTypes.Identifier).name, + ) && + args.length >= 1 && + containsUserInput(args[0]) + ) { + const method = (callee.property as BabelTypes.Identifier).name; + push(getNodeLine(path.node), `vm.${method}()`); + } + }, + + // ── new Function(..., userInput) ────────────────────────────────────── + NewExpression(path: NodePath) { + const callee = path.node.callee; + const args = path.node.arguments; + + if ( + callee.type === 'Identifier' && + (callee as BabelTypes.Identifier).name === 'Function' && + args.length >= 1 && + args.some(a => containsUserInput(a)) + ) { + push(getNodeLine(path.node), 'new Function()'); + } + + // new vm.Script(userInput) + if ( + callee.type === 'MemberExpression' && + callee.object.type === 'Identifier' && + (callee.object as BabelTypes.Identifier).name === 'vm' && + callee.property.type === 'Identifier' && + (callee.property as BabelTypes.Identifier).name === 'Script' && + args.length >= 1 && + containsUserInput(args[0]) + ) { + push(getNodeLine(path.node), 'new vm.Script()'); + } + }, + }); + + return findings; + }, +}; diff --git a/src/rules/validation/index.ts b/src/rules/validation/index.ts index 0152e27..6aa4787 100644 --- a/src/rules/validation/index.ts +++ b/src/rules/validation/index.ts @@ -1,12 +1,15 @@ export { unsafeReqBodyRule, unsafeQueryParamRule } from './input-validation.js'; export { prototypePollutionMergeRule } from './prototype-pollution.js'; +export { codeInjectionRule } from './code-injection.js'; import { unsafeReqBodyRule, unsafeQueryParamRule } from './input-validation.js'; import { prototypePollutionMergeRule } from './prototype-pollution.js'; +import { codeInjectionRule } from './code-injection.js'; import type { Rule } from '../../types/index.js'; export const validationRules: Rule[] = [ unsafeReqBodyRule, unsafeQueryParamRule, prototypePollutionMergeRule, + codeInjectionRule, ]; diff --git a/tests/rules/validation.test.ts b/tests/rules/validation.test.ts index 6722b47..bc1537f 100644 --- a/tests/rules/validation.test.ts +++ b/tests/rules/validation.test.ts @@ -120,3 +120,91 @@ describe('PP001 – Computed property assignment', () => { expect(prototypePollutionMergeRule.run(ctx)).toHaveLength(0); }); }); + +// --------------------------------------------------------------------------- +// INJECT001 – Code injection via eval / new Function / vm module +// --------------------------------------------------------------------------- +import { codeInjectionRule } from '../../src/rules/validation/code-injection.js'; + +describe('INJECT001 – eval with user input', () => { + it('flags eval(req.body.code)', () => { + const ctx = createContext(`eval(req.body.code);`); + const findings = codeInjectionRule.run(ctx); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe('INJECT001'); + expect(findings[0].severity).toBe('critical'); + }); + + it('flags eval(req.query.expr)', () => { + const ctx = createContext(`eval(req.query.expr);`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('flags eval with template literal containing user input', () => { + const ctx = createContext('eval(`return ${req.body.fn}`)'); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('flags eval with string concatenation of user input', () => { + const ctx = createContext(`eval('(' + req.body.code + ')');`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('does not flag eval with a hardcoded string', () => { + const ctx = createContext(`eval('1 + 1');`); + expect(codeInjectionRule.run(ctx)).toHaveLength(0); + }); + + it('does not flag eval with no arguments', () => { + const ctx = createContext(`eval();`); + expect(codeInjectionRule.run(ctx)).toHaveLength(0); + }); +}); + +describe('INJECT001 – new Function with user input', () => { + it('flags new Function(req.body.code)', () => { + const ctx = createContext(`const fn = new Function(req.body.code);`); + const findings = codeInjectionRule.run(ctx); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe('INJECT001'); + }); + + it('flags new Function with user input as last (body) argument', () => { + const ctx = createContext(`const fn = new Function('x', 'y', req.body.expr);`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('flags new Function with req.query', () => { + const ctx = createContext(`new Function(req.query.fn)();`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('does not flag new Function with only string literals', () => { + const ctx = createContext(`const fn = new Function('a', 'b', 'return a + b');`); + expect(codeInjectionRule.run(ctx)).toHaveLength(0); + }); +}); + +describe('INJECT001 – vm module with user input', () => { + it('flags vm.runInNewContext(req.body.script)', () => { + const ctx = createContext(`vm.runInNewContext(req.body.script, sandbox);`); + const findings = codeInjectionRule.run(ctx); + expect(findings).toHaveLength(1); + expect(findings[0].ruleId).toBe('INJECT001'); + }); + + it('flags vm.runInThisContext(req.query.code)', () => { + const ctx = createContext(`vm.runInThisContext(req.query.code);`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('flags new vm.Script(req.body.src)', () => { + const ctx = createContext(`const script = new vm.Script(req.body.src);`); + expect(codeInjectionRule.run(ctx)).toHaveLength(1); + }); + + it('does not flag vm.runInNewContext with a hardcoded string', () => { + const ctx = createContext(`vm.runInNewContext('1 + 1', {});`); + expect(codeInjectionRule.run(ctx)).toHaveLength(0); + }); +});