From 49686042707ca770330125dc5694d782898d9551 Mon Sep 17 00:00:00 2001 From: azizur100389 Date: Thu, 16 Jul 2026 13:11:57 +0100 Subject: [PATCH] feat(taint): expand TS/JS sink model (#2490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * feat(taint): expand TS/JS sink model Co-authored-by: Cursor * test(taint): cover TS sink disambiguation end-to-end Add a real-pipeline integration test proving the expanded TS/JS taint sinks only emit findings for intended imported and receiver-conventional symbols. Co-authored-by: Cursor --------- Co-authored-by: Cursor Co-authored-by: Gergő Magyar --- .../core/ingestion/taint/typescript-model.ts | 36 ++++++++- .../test/integration/taint-explain.test.ts | 77 +++++++++++++++++++ gitnexus/test/unit/taint/model-match.test.ts | 41 ++++++++++ gitnexus/test/unit/taint/propagate.test.ts | 48 ++++++++++++ 4 files changed, 200 insertions(+), 2 deletions(-) diff --git a/gitnexus/src/core/ingestion/taint/typescript-model.ts b/gitnexus/src/core/ingestion/taint/typescript-model.ts index bfa41c49c..6ef39cca9 100644 --- a/gitnexus/src/core/ingestion/taint/typescript-model.ts +++ b/gitnexus/src/core/ingestion/taint/typescript-model.ts @@ -38,10 +38,14 @@ export const TS_JS_TAINT_MODEL: SourceSinkSanitizerSpec = { }, ], sinks: [ - // Command execution — the command string is argument 0. + // Command execution — shell strings are arg 0; argv-form APIs also treat + // the argv array at arg 1 as command/option injection surface. { name: 'exec', kind: 'command-injection', args: [0], module: 'child_process' }, { name: 'execSync', kind: 'command-injection', args: [0], module: 'child_process' }, - { name: 'spawn', kind: 'command-injection', args: [0], module: 'child_process' }, + { name: 'spawn', kind: 'command-injection', args: [0, 1], module: 'child_process' }, + { name: 'spawnSync', kind: 'command-injection', args: [0, 1], module: 'child_process' }, + { name: 'execFile', kind: 'command-injection', args: [0, 1], module: 'child_process' }, + { name: 'execFileSync', kind: 'command-injection', args: [0, 1], module: 'child_process' }, // Code evaluation. `eval` takes code at 0; `new Function(...)` treats // EVERY argument as source text (params + body), so `args` is omitted // (= all positions) rather than pinned to 0. @@ -56,9 +60,37 @@ export const TS_JS_TAINT_MODEL: SourceSinkSanitizerSpec = { // (mysql2/pg/knex handles go by many names; receiver-conventional). { name: 'query', kind: 'sql-injection', args: [0], anyReceiver: true }, { name: 'execute', kind: 'sql-injection', args: [0], anyReceiver: true }, + // Modern DB libraries expose shorter method names with high collision + // rates (`map.get`, `task.run`, ...), so keep these receiver-conventional. + { + name: 'run', + kind: 'sql-injection', + args: [0], + receivers: ['db', 'database', 'conn', 'client', 'pool', 'stmt', 'statement', 'prepared'], + }, + { + name: 'all', + kind: 'sql-injection', + args: [0], + receivers: ['db', 'database', 'conn', 'client', 'pool', 'stmt', 'statement', 'prepared'], + }, + { + name: 'get', + kind: 'sql-injection', + args: [0], + receivers: ['db', 'database', 'conn', 'client', 'pool', 'stmt', 'statement', 'prepared'], + }, + { + name: 'values', + kind: 'sql-injection', + args: [0], + receivers: ['db', 'database', 'conn', 'client', 'pool'], + }, + { name: 'raw', kind: 'sql-injection', args: [0], receivers: ['db', 'knex', 'sequelize'] }, // Reflected XSS — Express response writes, conventional receiver `res`. { name: 'send', kind: 'xss', args: [0], receivers: ['res'] }, { name: 'write', kind: 'xss', args: [0], receivers: ['res'] }, + { name: 'render', kind: 'xss', args: [0, 1], receivers: ['res'] }, ], sanitizers: [ // URL-encoding: neutralizes markup injection AND path separators diff --git a/gitnexus/test/integration/taint-explain.test.ts b/gitnexus/test/integration/taint-explain.test.ts index 7f500679b..28478051e 100644 --- a/gitnexus/test/integration/taint-explain.test.ts +++ b/gitnexus/test/integration/taint-explain.test.ts @@ -26,6 +26,7 @@ import { LocalBackend } from '../../src/mcp/local/local-backend.js'; import { listRegisteredRepos, loadMeta } from '../../src/storage/repo-manager.js'; import { withTestLbugDB } from '../helpers/test-indexed-db.js'; import { runPipelineFromRepo } from '../../src/core/ingestion/pipeline.js'; +import { decodeTaintPath } from '../../src/core/ingestion/taint/path-codec.js'; vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { const actual = await importOriginal(); @@ -42,6 +43,82 @@ vi.mock('../../src/storage/repo-manager.js', async (importOriginal) => { const FIXTURE = path.join(__dirname, 'cfg', 'fixtures', 'pdg-repo'); +describe('TS/JS taint model sink disambiguation — real pipeline', () => { + it('emits findings only for the intended imported/receiver-conventional sinks', async () => { + const repoDir = fs.mkdtempSync(path.join(os.tmpdir(), 'gn-taint-disambig-')); + try { + fs.mkdirSync(path.join(repoDir, 'src'), { recursive: true }); + fs.writeFileSync( + path.join(repoDir, 'src', 'app.ts'), + `import { execFile as childExecFile, spawnSync } from 'node:child_process'; + +function execFile(cmd: string, args: string[]) { + return { cmd, args }; +} + +export function handle(req: any, db: any, map: Map, task: any, res: any, out: any) { + const value = req.body; + childExecFile('git', [value]); + spawnSync(value, []); + execFile(value, [value]); + db.run(value); + map.get(value); + task.run(value); + res.render(value, {}); + out.render(value, {}); +} +`, + ); + + const result = await runPipelineFromRepo(repoDir, () => {}, { pdg: true }); + const findings = [...result.graph.iterRelationships()] + .filter((rel) => rel.type === 'TAINTED') + .map((rel) => { + const sink = result.graph.getNode(rel.targetId); + const decoded = decodeTaintPath(rel.reason); + if (!decoded.ok) { + throw new Error(`invalid TAINTED reason for ${rel.id}: ${decoded.error}`); + } + return { + kind: decoded.kind, + sinkLine: decoded.hops.at(-1)?.line, + sinkText: String(sink?.properties.text ?? ''), + }; + }); + + expect(findings.map((f) => f.kind).sort()).toEqual([ + 'command-injection', + 'command-injection', + 'sql-injection', + 'xss', + ]); + const findingSites = findings + .map((f) => `${f.kind}@${f.sinkLine}`) + .sort((a, b) => Number(a.split('@')[1]) - Number(b.split('@')[1])); + expect(findingSites).toEqual([ + 'command-injection@9', + 'command-injection@10', + 'sql-injection@12', + 'xss@15', + ]); + expect(findings.map((f) => f.sinkLine).sort((a, b) => Number(a) - Number(b))).not.toContain( + 11, + ); + expect(findings.map((f) => f.sinkLine).sort((a, b) => Number(a) - Number(b))).not.toContain( + 13, + ); + expect(findings.map((f) => f.sinkLine).sort((a, b) => Number(a) - Number(b))).not.toContain( + 14, + ); + expect(findings.map((f) => f.sinkLine).sort((a, b) => Number(a) - Number(b))).not.toContain( + 16, + ); + } finally { + fs.rmSync(repoDir, { recursive: true, force: true }); + } + }); +}); + // ─── Block 1: a --pdg index with real taint findings ───────────────── withTestLbugDB( diff --git a/gitnexus/test/unit/taint/model-match.test.ts b/gitnexus/test/unit/taint/model-match.test.ts index 3c275eda7..7d89eeb98 100644 --- a/gitnexus/test/unit/taint/model-match.test.ts +++ b/gitnexus/test/unit/taint/model-match.test.ts @@ -99,6 +99,22 @@ function f(c) { execSync(c); }`); expect(allSinks(m).map((s) => s.entry.name)).toEqual(['execSync']); }); + it('argv-form child_process sinks match command injection on arg 0', () => { + const m = matchesOf(`import { execFile, execFileSync, spawnSync } from 'node:child_process'; +function f(cmd, arg) { + execFile(cmd, [arg]); + execFileSync(cmd, [arg]); + spawnSync(cmd, [arg]); +}`); + expect(allSinks(m).map((s) => s.entry.name)).toEqual(['execFile', 'execFileSync', 'spawnSync']); + expect(allSinks(m).map((s) => [...s.argPositions])).toEqual([ + [0, 1], + [0, 1], + [0, 1], + ]); + expect(allSinks(m).every((s) => s.entry.kind === 'command-injection')).toBe(true); + }); + it('an in-FUNCTION local `exec` shadows the import — no match', () => { const m = matchesOf(`import { exec } from 'child_process'; function f(c) { function exec(x) { return x; } exec(c); }`); @@ -225,10 +241,35 @@ describe('receiver-conventional sinks', () => { expect(allSinks(m).every((s) => s.entry.kind === 'xss')).toBe(true); }); + it('res.render matches template and data args; out.render does not', () => { + const m = matchesOf(`function f(res, out, template, data) { + res.render(template, data); + out.render(template, data); + }`); + const sinks = allSinks(m); + expect(sinks.map((s) => s.entry.name)).toEqual(['render']); + expect(sinks.map((s) => [...s.argPositions])).toEqual([[0, 1]]); + expect(sinks[0].entry.kind).toBe('xss'); + }); + it('.query/.execute match sql-injection on ANY receiver', () => { const m = matchesOf(`function f(db, pool, x) { db.query(x); pool.execute(x); }`); expect(allSinks(m).map((s) => s.entry.kind)).toEqual(['sql-injection', 'sql-injection']); }); + + it('modern DB method sinks match only conventional DB receivers', () => { + const m = matchesOf(`function f(db, stmt, knex, map, task, x) { + db.run(x); + db.all(x); + stmt.get(x); + knex.raw(x); + db.values(x); + map.get(x); + task.run(x); + }`); + expect(allSinks(m).map((s) => s.entry.name)).toEqual(['run', 'all', 'get', 'raw', 'values']); + expect(allSinks(m).every((s) => s.entry.kind === 'sql-injection')).toBe(true); + }); }); describe('sanitizers — import-aware only, kind-scoped', () => { diff --git a/gitnexus/test/unit/taint/propagate.test.ts b/gitnexus/test/unit/taint/propagate.test.ts index 2e8f9a549..a1bb61e3d 100644 --- a/gitnexus/test/unit/taint/propagate.test.ts +++ b/gitnexus/test/unit/taint/propagate.test.ts @@ -539,6 +539,54 @@ describe('multi-source identity — distinct sources do not merge at one def', ( // ── kind-set exclusion model (real built-in model) ────────────────────────── describe('kind-set exclusions — sanitizers neutralize their kinds only', () => { + it('built-in model catches argv-form child_process command sinks', () => { + const r = analyze( + `import { execFileSync } from 'node:child_process'; +function f(req) { + const tool = req.body; + const arg = req.query; + execFileSync(tool, ['--version']); + execFileSync('git', [arg]); +}`, + { spec: TS_JS_TAINT_MODEL }, + ); + expect(r.findings.map((finding) => finding.sinkKind)).toEqual([ + 'command-injection', + 'command-injection', + ]); + }); + + it('built-in model catches conventional modern DB receiver methods', () => { + const r = analyze( + `function f(req, db, stmt, knex) { + const name = req.body; + db.run(name); + db.all(name); + stmt.get(name); + knex.raw(name); +}`, + { spec: TS_JS_TAINT_MODEL }, + ); + expect(r.findings.map((finding) => finding.sinkKind)).toEqual([ + 'sql-injection', + 'sql-injection', + 'sql-injection', + 'sql-injection', + ]); + }); + + it('built-in model catches Express render template and data sinks', () => { + const r = analyze( + `function f(req, res) { + const template = req.body; + const viewData = req.query; + res.render(template, viewData); +}`, + { spec: TS_JS_TAINT_MODEL }, + ); + expect(r.findings.map((finding) => finding.sinkKind)).toEqual(['xss', 'xss']); + }); + it('escape(req.body) → res.send(b) suppressed (xss neutralized) BUT db.query(b) fires (sql not)', () => { const r = analyze( `import { escape } from 'validator';