From 303317692a2055f119ee4b29a41bf0925f5b5d88 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Sat, 9 Mar 2024 12:48:53 +0100 Subject: [PATCH 1/9] kickstart custom probe as param --- src/AstAnalyser.js | 5 ++- src/SourceFile.js | 10 ++++- test/issues/221-inject-custom-probes.spec.js | 40 ++++++++++++++++++++ test/utils/index.js | 3 +- 4 files changed, 53 insertions(+), 5 deletions(-) create mode 100644 test/issues/221-inject-custom-probes.spec.js diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index d957699..7855e21 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -16,8 +16,9 @@ export class AstAnalyser { * @constructor * @param { SourceParser } [parser] */ - constructor(parser = new JsSourceParser()) { + constructor(parser = new JsSourceParser(), customProbes = []) { this.parser = parser; + this.customProbes = customProbes; } analyse(str, options = Object.create(null)) { @@ -31,7 +32,7 @@ export class AstAnalyser { isEcmaScriptModule: Boolean(module) }); - const source = new SourceFile(str); + const source = new SourceFile(str, this.customProbes); // we walk each AST Nodes, this is a purely synchronous I/O walk(body, { diff --git a/src/SourceFile.js b/src/SourceFile.js index aa7513d..0fb670f 100644 --- a/src/SourceFile.js +++ b/src/SourceFile.js @@ -20,14 +20,20 @@ export class SourceFile { encodedLiterals = new Map(); warnings = []; - constructor(sourceCodeString) { + constructor(sourceCodeString, customProbes = []) { this.tracer = new VariableTracer() .enableDefaultTracing() .trace("crypto.createHash", { followConsecutiveAssignment: true, moduleName: "crypto" }); - this.probesRunner = new ProbeRunner(this); + if (Array.isArray(customProbes) && customProbes.length > 0) { + this.probesRunner = new ProbeRunner(this, customProbes); + } + else { + this.probesRunner = new ProbeRunner(this); + } + if (trojan.verify(sourceCodeString)) { this.addWarning("obfuscated-code", "trojan-source"); } diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js new file mode 100644 index 0000000..b229021 --- /dev/null +++ b/test/issues/221-inject-custom-probes.spec.js @@ -0,0 +1,40 @@ +// Import Node.js Dependencies +import { test } from "node:test"; +import assert from "node:assert"; + +// Import Internal Dependencies +import { JsSourceParser } from "../../src/JsSourceParser.js"; +import { AstAnalyser } from "../../src/AstAnalyser.js"; +import { ProbeSignals } from "../../src/ProbeRunner.js"; + +/** + * @see https://github.com/NodeSecure/js-x-ray/issues/221 + */ +// CONSTANTS +const kIncriminedCodeSample = "const danger = 'danger';"; +const kWarningUnsafeDanger = "unsafe-danger"; + +test("should detect a custom probe alert unsafe-danger", () => { + const customProbes = [ + { + name: "customProbeUnsafeDanger", + validateNode: (node, sourceFile) => [true] + , + main: (node, options) => { + const { sourceFile, data: calleeName } = options; + if (node.declarations[0].init.value === "danger") { + sourceFile.addWarning("unsafe-danger", calleeName, node.loc); + + return ProbeSignals.Skip; + } + + return null; + } + } + ]; + + const analyser = new AstAnalyser(new JsSourceParser(), customProbes); + const result = analyser.analyse(kIncriminedCodeSample); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); +}); diff --git a/test/utils/index.js b/test/utils/index.js index b6a170e..eb0873a 100644 --- a/test/utils/index.js +++ b/test/utils/index.js @@ -38,7 +38,8 @@ export function getSastAnalysis( return this.sourceFile.dependencies; }, execute(body) { - const probeRunner = new ProbeRunner(this.sourceFile, [probe]); + const probes = Array.isArray(probe) ? probe : [probe]; + const probeRunner = new ProbeRunner(this.sourceFile, probes); const self = this; walk(body, { From 5417a37c3c1eafc444a57d64032503f226d45436 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Sun, 10 Mar 2024 17:52:18 +0100 Subject: [PATCH 2/9] append/replace custom probe in ASTAnalyzer class --- src/AstAnalyser.js | 5 +- src/SourceFile.js | 8 ++-- test/issues/221-inject-custom-probes.spec.js | 49 +++++++++++++------- 3 files changed, 40 insertions(+), 22 deletions(-) diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index 7855e21..a279b71 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -16,9 +16,10 @@ export class AstAnalyser { * @constructor * @param { SourceParser } [parser] */ - constructor(parser = new JsSourceParser(), customProbes = []) { + constructor(parser = new JsSourceParser(), customProbes = [], mergeMode = "append") { this.parser = parser; this.customProbes = customProbes; + this.mergeMode = mergeMode; } analyse(str, options = Object.create(null)) { @@ -32,7 +33,7 @@ export class AstAnalyser { isEcmaScriptModule: Boolean(module) }); - const source = new SourceFile(str, this.customProbes); + const source = new SourceFile(str, this.customProbes, this.mergeMode); // we walk each AST Nodes, this is a purely synchronous I/O walk(body, { diff --git a/src/SourceFile.js b/src/SourceFile.js index 0fb670f..71bd3fb 100644 --- a/src/SourceFile.js +++ b/src/SourceFile.js @@ -20,19 +20,21 @@ export class SourceFile { encodedLiterals = new Map(); warnings = []; - constructor(sourceCodeString, customProbes = []) { + constructor(sourceCodeString, customProbes = [], mergeMode = "append") { this.tracer = new VariableTracer() .enableDefaultTracing() .trace("crypto.createHash", { followConsecutiveAssignment: true, moduleName: "crypto" }); + let mergedProbes; if (Array.isArray(customProbes) && customProbes.length > 0) { - this.probesRunner = new ProbeRunner(this, customProbes); + mergedProbes = mergeMode === "replace" ? customProbes : [...ProbeRunner.Defaults, ...customProbes]; } else { - this.probesRunner = new ProbeRunner(this); + mergedProbes = ProbeRunner.Defaults; } + this.probesRunner = new ProbeRunner(this, mergedProbes); if (trojan.verify(sourceCodeString)) { this.addWarning("obfuscated-code", "trojan-source"); diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js index b229021..ce60bac 100644 --- a/test/issues/221-inject-custom-probes.spec.js +++ b/test/issues/221-inject-custom-probes.spec.js @@ -11,30 +11,45 @@ import { ProbeSignals } from "../../src/ProbeRunner.js"; * @see https://github.com/NodeSecure/js-x-ray/issues/221 */ // CONSTANTS -const kIncriminedCodeSample = "const danger = 'danger';"; +const kIncriminedCodeSample = "const danger = 'danger'; const stream = eval('require')('stream');"; const kWarningUnsafeDanger = "unsafe-danger"; +const kWarningUnsafeImport = "unsafe-import"; +const kWarningUnsafeStmt = "unsafe-stmt"; -test("should detect a custom probe alert unsafe-danger", () => { - const customProbes = [ - { - name: "customProbeUnsafeDanger", - validateNode: (node, sourceFile) => [true] - , - main: (node, options) => { - const { sourceFile, data: calleeName } = options; - if (node.declarations[0].init.value === "danger") { - sourceFile.addWarning("unsafe-danger", calleeName, node.loc); - - return ProbeSignals.Skip; - } - - return null; +const customProbes = [ + { + name: "customProbeUnsafeDanger", + validateNode: (node, sourceFile) => { + return [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"]; + } + , + main: (node, options) => { + const { sourceFile, data: calleeName } = options; + if (node.declarations[0].init.value === "danger") { + sourceFile.addWarning("unsafe-danger", calleeName, node.loc); + + return ProbeSignals.Skip; } + + return null; } - ]; + } +]; +test("should append to list of probes (default)", () => { const analyser = new AstAnalyser(new JsSourceParser(), customProbes); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings[1].kind, kWarningUnsafeImport); + assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); + assert.equal(result.warnings.length, 3); +}); + +test("should replace list of probes", () => { + const analyser = new AstAnalyser(new JsSourceParser(), customProbes, "replace"); + const result = analyser.analyse(kIncriminedCodeSample); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings.length, 1); }); From 481da4007df67b30d20c4e8b71cc3aaa8f97af0d Mon Sep 17 00:00:00 2001 From: tchapacan Date: Mon, 11 Mar 2024 18:26:52 +0100 Subject: [PATCH 3/9] start refacto options param --- index.js | 6 ++-- src/AstAnalyser.js | 10 +++--- src/SourceFile.js | 13 +++---- test/AstAnalyser.spec.js | 8 ++++- test/issues/221-inject-custom-probes.spec.js | 22 ++++++++++-- test/utils/index.js | 3 +- types/api.d.ts | 36 +++++++++++++++++--- 7 files changed, 74 insertions(+), 24 deletions(-) diff --git a/index.js b/index.js index f6cfbec..0ea9988 100644 --- a/index.js +++ b/index.js @@ -9,10 +9,11 @@ function runASTAnalysis( ) { const { customParser = new JsSourceParser(), + astOptions = { isReplacing: false, customProbe: [] }, ...opts } = options; - const analyser = new AstAnalyser(customParser); + const analyser = new AstAnalyser(customParser, options.astOptions); return analyser.analyse(str, opts); } @@ -23,10 +24,11 @@ async function runASTAnalysisOnFile( ) { const { customParser = new JsSourceParser(), + astOptions = { isReplacing: false, customProbe: [] }, ...opts } = options; - const analyser = new AstAnalyser(customParser); + const analyser = new AstAnalyser(customParser, options.astOptions); return analyser.analyseFile(pathToFile, opts); } diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index a279b71..9b5b994 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -14,12 +14,12 @@ import { JsSourceParser } from "./JsSourceParser.js"; export class AstAnalyser { /** * @constructor - * @param { SourceParser } [parser] + * @param {SourceParser} [parser] + * @param astOptions */ - constructor(parser = new JsSourceParser(), customProbes = [], mergeMode = "append") { + constructor(parser = new JsSourceParser(), astOptions = { isReplacing: false, customProbe: [] }) { this.parser = parser; - this.customProbes = customProbes; - this.mergeMode = mergeMode; + this.astOptions = astOptions; } analyse(str, options = Object.create(null)) { @@ -33,7 +33,7 @@ export class AstAnalyser { isEcmaScriptModule: Boolean(module) }); - const source = new SourceFile(str, this.customProbes, this.mergeMode); + const source = new SourceFile(str, this.astOptions); // we walk each AST Nodes, this is a purely synchronous I/O walk(body, { diff --git a/src/SourceFile.js b/src/SourceFile.js index 71bd3fb..a680e21 100644 --- a/src/SourceFile.js +++ b/src/SourceFile.js @@ -20,21 +20,18 @@ export class SourceFile { encodedLiterals = new Map(); warnings = []; - constructor(sourceCodeString, customProbes = [], mergeMode = "append") { + constructor(sourceCodeString, astOptions = { isReplacing: false, customProbes: [] }) { this.tracer = new VariableTracer() .enableDefaultTracing() .trace("crypto.createHash", { followConsecutiveAssignment: true, moduleName: "crypto" }); - let mergedProbes; - if (Array.isArray(customProbes) && customProbes.length > 0) { - mergedProbes = mergeMode === "replace" ? customProbes : [...ProbeRunner.Defaults, ...customProbes]; + let probes = ProbeRunner.Defaults; + if (Array.isArray(astOptions.customProbes) && astOptions.customProbes.length > 0) { + probes = astOptions.isReplacing === true ? astOptions.customProbes : [...probes, ...astOptions.customProbes]; } - else { - mergedProbes = ProbeRunner.Defaults; - } - this.probesRunner = new ProbeRunner(this, mergedProbes); + this.probesRunner = new ProbeRunner(this, probes); if (trojan.verify(sourceCodeString)) { this.addWarning("obfuscated-code", "trojan-source"); diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index 3b792c3..f65c3f5 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -206,7 +206,7 @@ describe("AstAnalyser", (t) => { const preparedSource = getAnalyser().prepareSource(` `, { @@ -236,6 +236,12 @@ describe("AstAnalyser", (t) => { assert.deepEqual([...result.dependencies.keys()], []); }); }); + + it("should instantiate with correct default ASTOptions", () => { + const analyser = new AstAnalyser(); + assert.strictEqual(analyser.astOptions.isReplacing, false); + assert.deepStrictEqual(analyser.astOptions.customProbe, []); + }); }); }); diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js index ce60bac..779ea54 100644 --- a/test/issues/221-inject-custom-probes.spec.js +++ b/test/issues/221-inject-custom-probes.spec.js @@ -6,6 +6,7 @@ import assert from "node:assert"; import { JsSourceParser } from "../../src/JsSourceParser.js"; import { AstAnalyser } from "../../src/AstAnalyser.js"; import { ProbeSignals } from "../../src/ProbeRunner.js"; +import { runASTAnalysis } from "../../index.js"; /** * @see https://github.com/NodeSecure/js-x-ray/issues/221 @@ -37,7 +38,7 @@ const customProbes = [ ]; test("should append to list of probes (default)", () => { - const analyser = new AstAnalyser(new JsSourceParser(), customProbes); + const analyser = new AstAnalyser(new JsSourceParser(), { customProbes }); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); @@ -47,9 +48,26 @@ test("should append to list of probes (default)", () => { }); test("should replace list of probes", () => { - const analyser = new AstAnalyser(new JsSourceParser(), customProbes, "replace"); + const analyser = new AstAnalyser(new JsSourceParser(), { customProbes, isReplacing: true }); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings.length, 1); }); + + +test("should append list of probes using runASTAnalysis", () => { + const result = runASTAnalysis(kIncriminedCodeSample, { astOptions: { isReplacing: false, customProbes } }); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings[1].kind, kWarningUnsafeImport); + assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); + assert.equal(result.warnings.length, 3); +}); + +test("should replace list of probes using runASTAnalysis", () => { + const result = runASTAnalysis(kIncriminedCodeSample, { astOptions: { isReplacing: true, customProbes } }); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings.length, 1); +}); diff --git a/test/utils/index.js b/test/utils/index.js index eb0873a..b6a170e 100644 --- a/test/utils/index.js +++ b/test/utils/index.js @@ -38,8 +38,7 @@ export function getSastAnalysis( return this.sourceFile.dependencies; }, execute(body) { - const probes = Array.isArray(probe) ? probe : [probe]; - const probeRunner = new ProbeRunner(this.sourceFile, probes); + const probeRunner = new ProbeRunner(this.sourceFile, [probe]); const self = this; walk(body, { diff --git a/types/api.d.ts b/types/api.d.ts index f81ea66..a968cf7 100644 --- a/types/api.d.ts +++ b/types/api.d.ts @@ -1,5 +1,6 @@ import { Warning } from "./warnings.js"; import { Statement } from "meriyah/dist/src/estree.js"; +import {validateFunctionName} from "meriyah/dist/src/common"; export { AstAnalyser, @@ -34,6 +35,17 @@ interface Dependency { location?: null | SourceLocation; } +interface RootOptions { + /** + * @default ASTOptions + */ + ASTOptions?: ASTOptions; + /** + * @default RuntimeOptions + */ + RuntimeOptions?: RuntimeOptions; +} + interface RuntimeOptions { /** * @default true @@ -47,10 +59,26 @@ interface RuntimeOptions { * @default false */ removeHTMLComments?: boolean; - + customParser?: SourceParser; } +interface ASTOptions { + /** + * @default false + */ + isReplacing?: boolean; + /** + * @default [] + */ + customParser?: Probe[] | null; +} + +interface Probe { + validate: Function[] | Function; + main: Function[] | Function; +} + interface Report { dependencies: Map; warnings: Warning[]; @@ -78,10 +106,10 @@ interface SourceParser { } declare class AstAnalyser { - constructor(parser?: SourceParser); + constructor(parser?: SourceParser, astOptions?: ASTOptions); analyse: (str: string, options?: Omit) => Report; analyzeFile(pathToFile: string, options?: Omit): Promise; } -declare function runASTAnalysis(str: string, options?: RuntimeOptions): Report; -declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions): Promise; +declare function runASTAnalysis(str: string, options?: RootOptions): Report; +declare function runASTAnalysisOnFile(pathToFile: string, options?: RootOptions): Promise; From 69d53f64ae911ea7b3648b2ec822a7f89bd10589 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Thu, 14 Mar 2024 21:50:47 +0100 Subject: [PATCH 4/9] refacto ASTAnalyzer options to be a unique object --- index.js | 10 ++-- src/AstAnalyser.js | 26 +++++++--- src/SourceFile.js | 6 +-- test/AstAnalyser.spec.js | 25 +++++++-- test/issues/221-inject-custom-probes.spec.js | 19 ++++--- types/api.d.ts | 54 +++++++++----------- 6 files changed, 87 insertions(+), 53 deletions(-) diff --git a/index.js b/index.js index 0ea9988..e48a322 100644 --- a/index.js +++ b/index.js @@ -9,11 +9,12 @@ function runASTAnalysis( ) { const { customParser = new JsSourceParser(), - astOptions = { isReplacing: false, customProbe: [] }, + customProbe = [], + isReplacing = false, ...opts } = options; - const analyser = new AstAnalyser(customParser, options.astOptions); + const analyser = new AstAnalyser(options); return analyser.analyse(str, opts); } @@ -24,11 +25,12 @@ async function runASTAnalysisOnFile( ) { const { customParser = new JsSourceParser(), - astOptions = { isReplacing: false, customProbe: [] }, + customProbe = [], + isReplacing = false, ...opts } = options; - const analyser = new AstAnalyser(customParser, options.astOptions); + const analyser = new AstAnalyser(options); return analyser.analyseFile(pathToFile, opts); } diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index 9b5b994..3475c09 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -1,6 +1,7 @@ // Import Node.js Dependencies import fs from "node:fs/promises"; import path from "node:path"; +import assert from "node:assert"; // Import Third-party Dependencies import { walk } from "estree-walker"; @@ -14,12 +15,25 @@ import { JsSourceParser } from "./JsSourceParser.js"; export class AstAnalyser { /** * @constructor - * @param {SourceParser} [parser] - * @param astOptions + * @param options */ - constructor(parser = new JsSourceParser(), astOptions = { isReplacing: false, customProbe: [] }) { - this.parser = parser; - this.astOptions = astOptions; + constructor(options = {}) { + if (options.customParser !== undefined) { + assert(options.customParser instanceof JsSourceParser || typeof options.customParser === "object", + `customParser must be an instance of JsSourceParser or an object`); + } + if (options.customProbe !== undefined) { + assert(Array.isArray(options.customProbe), `customProbe must be an array`); + } + if (options.isReplacing !== undefined) { + assert(typeof options.isReplacing === "boolean", `isReplacing must be a boolean`); + } + + this.parser = options.customParser || new JsSourceParser(); + this.options = { + isReplacing: options.isReplacing || false, + customProbe: options.customProbe || [] + }; } analyse(str, options = Object.create(null)) { @@ -33,7 +47,7 @@ export class AstAnalyser { isEcmaScriptModule: Boolean(module) }); - const source = new SourceFile(str, this.astOptions); + const source = new SourceFile(str, this.options); // we walk each AST Nodes, this is a purely synchronous I/O walk(body, { diff --git a/src/SourceFile.js b/src/SourceFile.js index a680e21..bf4f9a4 100644 --- a/src/SourceFile.js +++ b/src/SourceFile.js @@ -20,7 +20,7 @@ export class SourceFile { encodedLiterals = new Map(); warnings = []; - constructor(sourceCodeString, astOptions = { isReplacing: false, customProbes: [] }) { + constructor(sourceCodeString, options = {}) { this.tracer = new VariableTracer() .enableDefaultTracing() .trace("crypto.createHash", { @@ -28,8 +28,8 @@ export class SourceFile { }); let probes = ProbeRunner.Defaults; - if (Array.isArray(astOptions.customProbes) && astOptions.customProbes.length > 0) { - probes = astOptions.isReplacing === true ? astOptions.customProbes : [...probes, ...astOptions.customProbes]; + if (Array.isArray(options.customProbe) && options.customProbe.length > 0) { + probes = options.isReplacing === true ? options.customProbe : [...probes, ...options.customProbe]; } this.probesRunner = new ProbeRunner(this, probes); diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index f65c3f5..3a404c0 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -1,6 +1,6 @@ // Import Node.js Dependencies import { describe, it } from "node:test"; -import assert from "node:assert"; +import assert, { AssertionError } from "node:assert"; import { readFileSync } from "node:fs"; // Import Internal Dependencies @@ -239,8 +239,27 @@ describe("AstAnalyser", (t) => { it("should instantiate with correct default ASTOptions", () => { const analyser = new AstAnalyser(); - assert.strictEqual(analyser.astOptions.isReplacing, false); - assert.deepStrictEqual(analyser.astOptions.customProbe, []); + assert(analyser.parser instanceof JsSourceParser || typeof analyser.parser.customParser === "object"); + assert.deepStrictEqual(analyser.options.customProbe, []); + assert.strictEqual(analyser.options.isReplacing, false); + }); + + it("should throw an error if customParser is not an instance of JsSourceParser", () => { + assert.throws(() => { + new AstAnalyser({ customParser: "new JsSourceParser()" }); + }, AssertionError); + }); + + it("should throw an error if customProbe is not an array", () => { + assert.throws(() => { + new AstAnalyser({ customProbe: "notArray" }); + }, AssertionError); + }); + + it("should throw an error if isReplacing is not a boolean", () => { + assert.throws(() => { + new AstAnalyser({ isReplacing: "false" }); + }, AssertionError); }); }); }); diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js index 779ea54..287250f 100644 --- a/test/issues/221-inject-custom-probes.spec.js +++ b/test/issues/221-inject-custom-probes.spec.js @@ -20,9 +20,7 @@ const kWarningUnsafeStmt = "unsafe-stmt"; const customProbes = [ { name: "customProbeUnsafeDanger", - validateNode: (node, sourceFile) => { - return [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"]; - } + validateNode: (node, sourceFile) => [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"] , main: (node, options) => { const { sourceFile, data: calleeName } = options; @@ -38,7 +36,7 @@ const customProbes = [ ]; test("should append to list of probes (default)", () => { - const analyser = new AstAnalyser(new JsSourceParser(), { customProbes }); + const analyser = new AstAnalyser({ parser: new JsSourceParser(), customProbe: customProbes }); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); @@ -48,16 +46,18 @@ test("should append to list of probes (default)", () => { }); test("should replace list of probes", () => { - const analyser = new AstAnalyser(new JsSourceParser(), { customProbes, isReplacing: true }); + const analyser = new AstAnalyser({ parser: new JsSourceParser(), customProbe: customProbes, isReplacing: true }); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings.length, 1); }); - test("should append list of probes using runASTAnalysis", () => { - const result = runASTAnalysis(kIncriminedCodeSample, { astOptions: { isReplacing: false, customProbes } }); + const result = runASTAnalysis( + kIncriminedCodeSample, + { parser: new JsSourceParser(), customProbe: customProbes, isReplacing: false } + ); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings[1].kind, kWarningUnsafeImport); @@ -66,7 +66,10 @@ test("should append list of probes using runASTAnalysis", () => { }); test("should replace list of probes using runASTAnalysis", () => { - const result = runASTAnalysis(kIncriminedCodeSample, { astOptions: { isReplacing: true, customProbes } }); + const result = runASTAnalysis( + kIncriminedCodeSample, + { parser: new JsSourceParser(), customProbe: customProbes, isReplacing: true } + ); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings.length, 1); diff --git a/types/api.d.ts b/types/api.d.ts index a968cf7..0ec2039 100644 --- a/types/api.d.ts +++ b/types/api.d.ts @@ -1,6 +1,5 @@ import { Warning } from "./warnings.js"; import { Statement } from "meriyah/dist/src/estree.js"; -import {validateFunctionName} from "meriyah/dist/src/common"; export { AstAnalyser, @@ -35,18 +34,7 @@ interface Dependency { location?: null | SourceLocation; } -interface RootOptions { - /** - * @default ASTOptions - */ - ASTOptions?: ASTOptions; - /** - * @default RuntimeOptions - */ - RuntimeOptions?: RuntimeOptions; -} - -interface RuntimeOptions { +interface RuntimeCommonOptions { /** * @default true */ @@ -54,26 +42,38 @@ interface RuntimeOptions { /** * @default false */ - isMinified?: boolean; + removeHTMLComments?: boolean; +} + +interface RuntimeDefaultOptions extends RuntimeCommonOptions { /** * @default false */ - removeHTMLComments?: boolean; + isMinified?: boolean; +} - customParser?: SourceParser; +interface RuntimeFileOptions extends RuntimeCommonOptions { + packageName?: string; } -interface ASTOptions { +interface RuntimeAnalyzerOptions { /** - * @default false + * @default JsSourceParser */ - isReplacing?: boolean; + customParser?: SourceParser; /** * @default [] */ - customParser?: Probe[] | null; + customProbe?: Probe[] | null; + /** + * @default false + */ + isReplacing?: boolean; } +type RuntimeOptions = RuntimeAnalyzerOptions & (RuntimeDefaultOptions | RuntimeFileOptions); + + interface Probe { validate: Function[] | Function; main: Function[] | Function; @@ -87,10 +87,6 @@ interface Report { isOneLineRequire: boolean; } -interface RuntimeFileOptions extends Omit { - packageName?: string; -} - type ReportOnFile = { ok: true, warnings: Warning[]; @@ -106,10 +102,10 @@ interface SourceParser { } declare class AstAnalyser { - constructor(parser?: SourceParser, astOptions?: ASTOptions); - analyse: (str: string, options?: Omit) => Report; - analyzeFile(pathToFile: string, options?: Omit): Promise; + constructor(options?: RuntimeOptions); + analyse: (str: string, options?: RuntimeDefaultOptions) => Report; + analyzeFile(pathToFile: string, options?: RuntimeFileOptions): Promise; } -declare function runASTAnalysis(str: string, options?: RootOptions): Report; -declare function runASTAnalysisOnFile(pathToFile: string, options?: RootOptions): Promise; +declare function runASTAnalysis(str: string, options?: RuntimeOptions): Report; +declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeOptions): Promise; From a1dd4bff1b2f99af14b918285647c1afb1405762 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Sun, 17 Mar 2024 23:06:18 +0100 Subject: [PATCH 5/9] refacto probesOptions ASTAnalyzer --- index.js | 8 +++--- src/AstAnalyser.js | 26 +++++++------------- src/SourceFile.js | 6 ++--- test/AstAnalyser.spec.js | 22 ++--------------- test/issues/221-inject-custom-probes.spec.js | 22 ++++++++++++++--- types/api.d.ts | 4 +-- 6 files changed, 38 insertions(+), 50 deletions(-) diff --git a/index.js b/index.js index e48a322..b3fae03 100644 --- a/index.js +++ b/index.js @@ -9,8 +9,8 @@ function runASTAnalysis( ) { const { customParser = new JsSourceParser(), - customProbe = [], - isReplacing = false, + customProbes = [], + skipDefaultProbes = false, ...opts } = options; @@ -25,8 +25,8 @@ async function runASTAnalysisOnFile( ) { const { customParser = new JsSourceParser(), - customProbe = [], - isReplacing = false, + customProbes = [], + skipDefaultProbes = false, ...opts } = options; diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index 3475c09..49b4c0a 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -15,24 +15,16 @@ import { JsSourceParser } from "./JsSourceParser.js"; export class AstAnalyser { /** * @constructor - * @param options + * @param {object} [options={}] + * @param {JsSourceParser} [options.customParser] + * @param {Array} [options.customProbes] + * @param {boolean} [options.skipDefaultProbes=false] */ constructor(options = {}) { - if (options.customParser !== undefined) { - assert(options.customParser instanceof JsSourceParser || typeof options.customParser === "object", - `customParser must be an instance of JsSourceParser or an object`); - } - if (options.customProbe !== undefined) { - assert(Array.isArray(options.customProbe), `customProbe must be an array`); - } - if (options.isReplacing !== undefined) { - assert(typeof options.isReplacing === "boolean", `isReplacing must be a boolean`); - } - - this.parser = options.customParser || new JsSourceParser(); - this.options = { - isReplacing: options.isReplacing || false, - customProbe: options.customProbe || [] + this.parser = options.customParser ?? new JsSourceParser(); + this.probesOptions = { + customProbes: options.customProbes ?? [], + skipDefaultProbes: options.skipDefaultProbes ?? false }; } @@ -47,7 +39,7 @@ export class AstAnalyser { isEcmaScriptModule: Boolean(module) }); - const source = new SourceFile(str, this.options); + const source = new SourceFile(str, this.probesOptions); // we walk each AST Nodes, this is a purely synchronous I/O walk(body, { diff --git a/src/SourceFile.js b/src/SourceFile.js index bf4f9a4..2c3e02b 100644 --- a/src/SourceFile.js +++ b/src/SourceFile.js @@ -20,7 +20,7 @@ export class SourceFile { encodedLiterals = new Map(); warnings = []; - constructor(sourceCodeString, options = {}) { + constructor(sourceCodeString, probesOptions = {}) { this.tracer = new VariableTracer() .enableDefaultTracing() .trace("crypto.createHash", { @@ -28,8 +28,8 @@ export class SourceFile { }); let probes = ProbeRunner.Defaults; - if (Array.isArray(options.customProbe) && options.customProbe.length > 0) { - probes = options.isReplacing === true ? options.customProbe : [...probes, ...options.customProbe]; + if (Array.isArray(probesOptions.customProbes) && probesOptions.customProbes.length > 0) { + probes = probesOptions.skipDefaultProbes === true ? probesOptions.customProbes : [...probes, ...probesOptions.customProbes]; } this.probesRunner = new ProbeRunner(this, probes); diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index 3a404c0..c9b4597 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -240,26 +240,8 @@ describe("AstAnalyser", (t) => { it("should instantiate with correct default ASTOptions", () => { const analyser = new AstAnalyser(); assert(analyser.parser instanceof JsSourceParser || typeof analyser.parser.customParser === "object"); - assert.deepStrictEqual(analyser.options.customProbe, []); - assert.strictEqual(analyser.options.isReplacing, false); - }); - - it("should throw an error if customParser is not an instance of JsSourceParser", () => { - assert.throws(() => { - new AstAnalyser({ customParser: "new JsSourceParser()" }); - }, AssertionError); - }); - - it("should throw an error if customProbe is not an array", () => { - assert.throws(() => { - new AstAnalyser({ customProbe: "notArray" }); - }, AssertionError); - }); - - it("should throw an error if isReplacing is not a boolean", () => { - assert.throws(() => { - new AstAnalyser({ isReplacing: "false" }); - }, AssertionError); + assert.deepStrictEqual(analyser.probesOptions.customProbes, []); + assert.strictEqual(analyser.probesOptions.skipDefaultProbes, false); }); }); }); diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js index 287250f..255466b 100644 --- a/test/issues/221-inject-custom-probes.spec.js +++ b/test/issues/221-inject-custom-probes.spec.js @@ -36,7 +36,7 @@ const customProbes = [ ]; test("should append to list of probes (default)", () => { - const analyser = new AstAnalyser({ parser: new JsSourceParser(), customProbe: customProbes }); + const analyser = new AstAnalyser({ customParser: new JsSourceParser(), customProbes }); const result = analyser.analyse(kIncriminedCodeSample); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); @@ -46,9 +46,15 @@ test("should append to list of probes (default)", () => { }); test("should replace list of probes", () => { - const analyser = new AstAnalyser({ parser: new JsSourceParser(), customProbe: customProbes, isReplacing: true }); + const analyser = new AstAnalyser({ + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: true + }); const result = analyser.analyse(kIncriminedCodeSample); + console.log(analyser); + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings.length, 1); }); @@ -56,7 +62,11 @@ test("should replace list of probes", () => { test("should append list of probes using runASTAnalysis", () => { const result = runASTAnalysis( kIncriminedCodeSample, - { parser: new JsSourceParser(), customProbe: customProbes, isReplacing: false } + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: false + } ); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); @@ -68,7 +78,11 @@ test("should append list of probes using runASTAnalysis", () => { test("should replace list of probes using runASTAnalysis", () => { const result = runASTAnalysis( kIncriminedCodeSample, - { parser: new JsSourceParser(), customProbe: customProbes, isReplacing: true } + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: true + } ); assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); diff --git a/types/api.d.ts b/types/api.d.ts index 0ec2039..ce1d999 100644 --- a/types/api.d.ts +++ b/types/api.d.ts @@ -64,11 +64,11 @@ interface RuntimeAnalyzerOptions { /** * @default [] */ - customProbe?: Probe[] | null; + customProbes?: Probe[] | null; /** * @default false */ - isReplacing?: boolean; + skipDefaultProbes?: boolean; } type RuntimeOptions = RuntimeAnalyzerOptions & (RuntimeDefaultOptions | RuntimeFileOptions); From ec738349cfd41795e10ca1c88ee8134b785a7644 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Tue, 19 Mar 2024 20:30:21 +0100 Subject: [PATCH 6/9] fix last typos --- index.js | 12 +++++++-- src/AstAnalyser.js | 3 +-- test/AstAnalyser.spec.js | 2 +- test/issues/221-inject-custom-probes.spec.js | 2 -- types/api.d.ts | 26 ++++++++------------ 5 files changed, 22 insertions(+), 23 deletions(-) diff --git a/index.js b/index.js index b3fae03..0702198 100644 --- a/index.js +++ b/index.js @@ -14,7 +14,11 @@ function runASTAnalysis( ...opts } = options; - const analyser = new AstAnalyser(options); + const analyser = new AstAnalyser({ + customParser, + customProbes, + skipDefaultProbes + }); return analyser.analyse(str, opts); } @@ -30,7 +34,11 @@ async function runASTAnalysisOnFile( ...opts } = options; - const analyser = new AstAnalyser(options); + const analyser = new AstAnalyser({ + customParser, + customProbes, + skipDefaultProbes + }); return analyser.analyseFile(pathToFile, opts); } diff --git a/src/AstAnalyser.js b/src/AstAnalyser.js index 49b4c0a..15e8a72 100644 --- a/src/AstAnalyser.js +++ b/src/AstAnalyser.js @@ -1,7 +1,6 @@ // Import Node.js Dependencies import fs from "node:fs/promises"; import path from "node:path"; -import assert from "node:assert"; // Import Third-party Dependencies import { walk } from "estree-walker"; @@ -16,7 +15,7 @@ export class AstAnalyser { /** * @constructor * @param {object} [options={}] - * @param {JsSourceParser} [options.customParser] + * @param {SourceParser} [options.customParser] * @param {Array} [options.customProbes] * @param {boolean} [options.skipDefaultProbes=false] */ diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index c9b4597..d1fe034 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -1,6 +1,6 @@ // Import Node.js Dependencies import { describe, it } from "node:test"; -import assert, { AssertionError } from "node:assert"; +import assert from "node:assert"; import { readFileSync } from "node:fs"; // Import Internal Dependencies diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js index 255466b..4a6b3c9 100644 --- a/test/issues/221-inject-custom-probes.spec.js +++ b/test/issues/221-inject-custom-probes.spec.js @@ -53,8 +53,6 @@ test("should replace list of probes", () => { }); const result = analyser.analyse(kIncriminedCodeSample); - console.log(analyser); - assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); assert.equal(result.warnings.length, 1); }); diff --git a/types/api.d.ts b/types/api.d.ts index ce1d999..27d7ac4 100644 --- a/types/api.d.ts +++ b/types/api.d.ts @@ -34,7 +34,7 @@ interface Dependency { location?: null | SourceLocation; } -interface RuntimeCommonOptions { +interface RuntimeOptions { /** * @default true */ @@ -43,20 +43,17 @@ interface RuntimeCommonOptions { * @default false */ removeHTMLComments?: boolean; -} - -interface RuntimeDefaultOptions extends RuntimeCommonOptions { /** * @default false */ isMinified?: boolean; } -interface RuntimeFileOptions extends RuntimeCommonOptions { +interface RuntimeFileOptions extends Omit { packageName?: string; } -interface RuntimeAnalyzerOptions { +interface AstAnalyserOptions { /** * @default JsSourceParser */ @@ -64,19 +61,16 @@ interface RuntimeAnalyzerOptions { /** * @default [] */ - customProbes?: Probe[] | null; + customProbes?: Probe[]; /** * @default false */ skipDefaultProbes?: boolean; } -type RuntimeOptions = RuntimeAnalyzerOptions & (RuntimeDefaultOptions | RuntimeFileOptions); - - interface Probe { - validate: Function[] | Function; - main: Function[] | Function; + validateNode: Function | Function[]; + main: Function; } interface Report { @@ -102,10 +96,10 @@ interface SourceParser { } declare class AstAnalyser { - constructor(options?: RuntimeOptions); - analyse: (str: string, options?: RuntimeDefaultOptions) => Report; + constructor(options?: AstAnalyserOptions); + analyse: (str: string, options?: RuntimeOptions) => Report; analyzeFile(pathToFile: string, options?: RuntimeFileOptions): Promise; } -declare function runASTAnalysis(str: string, options?: RuntimeOptions): Report; -declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeOptions): Promise; +declare function runASTAnalysis(str: string, options?: RuntimeOptions & AstAnalyserOptions): Report; +declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions & AstAnalyserOptions): Promise; From 9776648de38a6f313c934ddbf33b45619074ee40 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Thu, 21 Mar 2024 00:25:32 +0100 Subject: [PATCH 7/9] refacto tests --- test/AstAnalyser.spec.js | 31 ++++++- .../searchRuntimeDependencies/customProbe.js | 2 + test/issues/221-inject-custom-probes.spec.js | 88 ------------------- test/runASTAnalysis.spec.js | 37 ++++++++ test/runASTAnalysisOnFile.spec.js | 31 +++++++ test/utils/index.js | 25 +++++- 6 files changed, 124 insertions(+), 90 deletions(-) create mode 100644 test/fixtures/searchRuntimeDependencies/customProbe.js delete mode 100644 test/issues/221-inject-custom-probes.spec.js diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index d1fe034..6736cc8 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -6,7 +6,14 @@ import { readFileSync } from "node:fs"; // Import Internal Dependencies import { AstAnalyser } from "../src/AstAnalyser.js"; import { JsSourceParser } from "../src/JsSourceParser.js"; -import { getWarningKind } from "./utils/index.js"; +import { + customProbes, + getWarningKind, + kIncriminedCodeSampleCustomProbe, + kWarningUnsafeDanger, + kWarningUnsafeImport, + kWarningUnsafeStmt +} from "./utils/index.js"; // CONSTANTS const FIXTURE_URL = new URL("fixtures/searchRuntimeDependencies/", import.meta.url); @@ -145,6 +152,28 @@ describe("AstAnalyser", (t) => { ["http", "fs", "xd"].sort() ); }); + + it("should append to list of probes (default)", () => { + const analyser = new AstAnalyser({ customParser: new JsSourceParser(), customProbes }); + const result = analyser.analyse(kIncriminedCodeSampleCustomProbe); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings[1].kind, kWarningUnsafeImport); + assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); + assert.equal(result.warnings.length, 3); + }); + + it("should replace list of probes", () => { + const analyser = new AstAnalyser({ + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: true + }); + const result = analyser.analyse(kIncriminedCodeSampleCustomProbe); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings.length, 1); + }); }); it("remove the packageName from the dependencies list", async() => { diff --git a/test/fixtures/searchRuntimeDependencies/customProbe.js b/test/fixtures/searchRuntimeDependencies/customProbe.js new file mode 100644 index 0000000..2cbced9 --- /dev/null +++ b/test/fixtures/searchRuntimeDependencies/customProbe.js @@ -0,0 +1,2 @@ +const danger = 'danger'; +const stream = eval('require')('stream'); diff --git a/test/issues/221-inject-custom-probes.spec.js b/test/issues/221-inject-custom-probes.spec.js deleted file mode 100644 index 4a6b3c9..0000000 --- a/test/issues/221-inject-custom-probes.spec.js +++ /dev/null @@ -1,88 +0,0 @@ -// Import Node.js Dependencies -import { test } from "node:test"; -import assert from "node:assert"; - -// Import Internal Dependencies -import { JsSourceParser } from "../../src/JsSourceParser.js"; -import { AstAnalyser } from "../../src/AstAnalyser.js"; -import { ProbeSignals } from "../../src/ProbeRunner.js"; -import { runASTAnalysis } from "../../index.js"; - -/** - * @see https://github.com/NodeSecure/js-x-ray/issues/221 - */ -// CONSTANTS -const kIncriminedCodeSample = "const danger = 'danger'; const stream = eval('require')('stream');"; -const kWarningUnsafeDanger = "unsafe-danger"; -const kWarningUnsafeImport = "unsafe-import"; -const kWarningUnsafeStmt = "unsafe-stmt"; - -const customProbes = [ - { - name: "customProbeUnsafeDanger", - validateNode: (node, sourceFile) => [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"] - , - main: (node, options) => { - const { sourceFile, data: calleeName } = options; - if (node.declarations[0].init.value === "danger") { - sourceFile.addWarning("unsafe-danger", calleeName, node.loc); - - return ProbeSignals.Skip; - } - - return null; - } - } -]; - -test("should append to list of probes (default)", () => { - const analyser = new AstAnalyser({ customParser: new JsSourceParser(), customProbes }); - const result = analyser.analyse(kIncriminedCodeSample); - - assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); - assert.equal(result.warnings[1].kind, kWarningUnsafeImport); - assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); - assert.equal(result.warnings.length, 3); -}); - -test("should replace list of probes", () => { - const analyser = new AstAnalyser({ - parser: new JsSourceParser(), - customProbes, - skipDefaultProbes: true - }); - const result = analyser.analyse(kIncriminedCodeSample); - - assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); - assert.equal(result.warnings.length, 1); -}); - -test("should append list of probes using runASTAnalysis", () => { - const result = runASTAnalysis( - kIncriminedCodeSample, - { - parser: new JsSourceParser(), - customProbes, - skipDefaultProbes: false - } - ); - - assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); - assert.equal(result.warnings[1].kind, kWarningUnsafeImport); - assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); - assert.equal(result.warnings.length, 3); -}); - -test("should replace list of probes using runASTAnalysis", () => { - const result = runASTAnalysis( - kIncriminedCodeSample, - { - parser: new JsSourceParser(), - customProbes, - skipDefaultProbes: true - } - ); - - assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); - assert.equal(result.warnings.length, 1); -}); diff --git a/test/runASTAnalysis.spec.js b/test/runASTAnalysis.spec.js index 318ff24..0cc6660 100644 --- a/test/runASTAnalysis.spec.js +++ b/test/runASTAnalysis.spec.js @@ -7,6 +7,13 @@ import { runASTAnalysis } from "../index.js"; import { AstAnalyser } from "../src/AstAnalyser.js"; import { JsSourceParser } from "../src/JsSourceParser.js"; import { FakeSourceParser } from "./fixtures/FakeSourceParser.js"; +import { + customProbes, + kIncriminedCodeSampleCustomProbe, + kWarningUnsafeDanger, + kWarningUnsafeImport, + kWarningUnsafeStmt +} from "./utils/index.js"; it("should call AstAnalyser.analyse with the expected arguments", (t) => { t.mock.method(AstAnalyser.prototype, "analyse"); @@ -37,3 +44,33 @@ it("should instantiate AstAnalyser with the expected parser", (t) => { assert.strictEqual(JsSourceParser.prototype.parse.mock.calls.length, 1); assert.strictEqual(FakeSourceParser.prototype.parse.mock.calls.length, 1); }); + +it("should append list of probes using runASTAnalysis", () => { + const result = runASTAnalysis( + kIncriminedCodeSampleCustomProbe, + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: false + } + ); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings[1].kind, kWarningUnsafeImport); + assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); + assert.equal(result.warnings.length, 3); +}); + +it("should replace list of probes using runASTAnalysis", () => { + const result = runASTAnalysis( + kIncriminedCodeSampleCustomProbe, + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: true + } + ); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings.length, 1); +}); diff --git a/test/runASTAnalysisOnFile.spec.js b/test/runASTAnalysisOnFile.spec.js index 42a018d..bb8047a 100644 --- a/test/runASTAnalysisOnFile.spec.js +++ b/test/runASTAnalysisOnFile.spec.js @@ -7,6 +7,7 @@ import { runASTAnalysisOnFile } from "../index.js"; import { AstAnalyser } from "../src/AstAnalyser.js"; import { FakeSourceParser } from "./fixtures/FakeSourceParser.js"; import { JsSourceParser } from "../src/JsSourceParser.js"; +import { customProbes, kWarningUnsafeDanger, kWarningUnsafeImport, kWarningUnsafeStmt } from "./utils/index.js"; // CONSTANTS const FIXTURE_URL = new URL("fixtures/searchRuntimeDependencies/", import.meta.url); @@ -50,3 +51,33 @@ it("should instantiate AstAnalyser with the expected parser", async(t) => { assert.strictEqual(JsSourceParser.prototype.parse.mock.calls.length, 1); assert.strictEqual(FakeSourceParser.prototype.parse.mock.calls.length, 1); }); + +it("should append list of probes using runASTAnalysisOnFile", async() => { + const result = await runASTAnalysisOnFile( + new URL("customProbe.js", FIXTURE_URL), + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: false + } + ); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings[1].kind, kWarningUnsafeImport); + assert.equal(result.warnings[2].kind, kWarningUnsafeStmt); + assert.equal(result.warnings.length, 3); +}); + +it("should replace list of probes using runASTAnalysisOnFile", async() => { + const result = await runASTAnalysisOnFile( + new URL("customProbe.js", FIXTURE_URL), + { + parser: new JsSourceParser(), + customProbes, + skipDefaultProbes: true + } + ); + + assert.equal(result.warnings[0].kind, kWarningUnsafeDanger); + assert.equal(result.warnings.length, 1); +}); diff --git a/test/utils/index.js b/test/utils/index.js index b6a170e..722c598 100644 --- a/test/utils/index.js +++ b/test/utils/index.js @@ -4,7 +4,7 @@ import { walk } from "estree-walker"; // Import Internal Dependencies import { SourceFile } from "../../src/SourceFile.js"; -import { ProbeRunner } from "../../src/ProbeRunner.js"; +import { ProbeRunner, ProbeSignals } from "../../src/ProbeRunner.js"; export function getWarningKind(warnings) { return warnings.slice().map((warn) => warn.kind).sort(); @@ -61,3 +61,26 @@ export function getSastAnalysis( } }; } + +export const customProbes = [ + { + name: "customProbeUnsafeDanger", + validateNode: (node, sourceFile) => [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"] + , + main: (node, options) => { + const { sourceFile, data: calleeName } = options; + if (node.declarations[0].init.value === "danger") { + sourceFile.addWarning("unsafe-danger", calleeName, node.loc); + + return ProbeSignals.Skip; + } + + return null; + } + } +]; + +export const kIncriminedCodeSampleCustomProbe = "const danger = 'danger'; const stream = eval('require')('stream');"; +export const kWarningUnsafeDanger = "unsafe-danger"; +export const kWarningUnsafeImport = "unsafe-import"; +export const kWarningUnsafeStmt = "unsafe-stmt"; From f43a98cc23f76e1a349d3ade1f372eb134ce5f45 Mon Sep 17 00:00:00 2001 From: tchapacan Date: Mon, 25 Mar 2024 09:46:23 +0100 Subject: [PATCH 8/9] refacto tests && update docs --- README.md | 87 ++++++++++++++++++++++++++++++++++++++-- test/AstAnalyser.spec.js | 4 +- 2 files changed, 86 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 9079223..25e0c10 100644 --- a/README.md +++ b/README.md @@ -134,15 +134,88 @@ This section describe all the possible warnings returned by JSXRay. Click on the | [weak-crypto](./docs/weak-crypto.md) | ✔️ | The code probably contains a weak crypto algorithm (md5, sha1...) | | [shady-link](./docs/shady-link.md) | ✔️ | The code contains shady/unsafe link | +## Custom Probes + +You can also create custom probes to detect specific pattern in the code you are analyzing. + +A probe is a pair of two functions (`validateNode` and `main`) that will be called on each node of the AST. It will return a warning if the pattern is detected. +Below a basic probe that detect a string assignation to `danger`: + +```ts +export const customProbes = [ + { + name: "customProbeUnsafeDanger", + validateNode: (node, sourceFile) => [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"] + , + main: (node, options) => { + const { sourceFile, data: calleeName } = options; + if (node.declarations[0].init.value === "danger") { + sourceFile.addWarning("unsafe-danger", calleeName, node.loc); + + return ProbeSignals.Skip; + } + + return null; + } + } +]; +``` + +You can pass an array of probes to the `runASTAnalysis/runASTAnalysisOnFile` functions as `options`, or directly to the `AstAnalyser` constructor. + +| Name | Type | Description | Default Value | +|------------------|----------------------------------|-----------------------------------------------------------------------|-----------------| +| `customParser` | `SourceParser \| undefined` | An optional custom parser to be used for parsing the source code. | `JsSourceParser` | +| `customProbes` | `Probe[] \| undefined` | An array of custom probes to be used during AST analysis. | `[]` | +| `skipDefaultProbes` | `boolean \| undefined` | If `true`, default probes will be skipped and only custom probes will be used. | `false` | + + +Here using the example probe upper: + +```ts +import { runASTAnalysis } from "@nodesecure/js-x-ray"; + +// add your customProbes here (see example above) + +const result = runASTAnalysis("const danger = 'danger';", { customProbes, skipDefaultProbes: true }); + +console.log(result); +``` + +Result: + +```sh +✗ node example.js +{ + idsLengthAvg: 0, + stringScore: 0, + warnings: [ { kind: 'unsafe-danger', location: [Array], source: 'JS-X-Ray' } ], + dependencies: Map(0) {}, + isOneLineRequire: false +} +``` + +Congrats, you have created your first custom probe! 🎉 + +> Check the types in [index.d.ts](index.d.ts) and [types/api.d.ts](types/api.d.ts) for more details about the `options` +> ## API
-runASTAnalysis(str: string, options?: RuntimeOptions): Report +declare function runASTAnalysis(str: string, options?: RuntimeOptions & AstAnalyserOptions): Report ```ts interface RuntimeOptions { module?: boolean; - isMinified?: boolean; removeHTMLComments?: boolean; + isMinified?: boolean; +} +``` + +```ts +interface AstAnalyserOptions { + customParser?: SourceParser; + customProbes?: Probe[]; + skipDefaultProbes?: boolean; } ``` @@ -161,7 +234,7 @@ interface Report {
-runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions): Promise< ReportOnFile > +declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions & AstAnalyserOptions): Promise< ReportOnFile > ```ts interface RuntimeFileOptions { @@ -171,6 +244,14 @@ interface RuntimeFileOptions { } ``` +```ts +interface AstAnalyserOptions { + customParser?: SourceParser; + customProbes?: Probe[]; + skipDefaultProbes?: boolean; +} +``` + Run the SAST scanner on a given JavaScript file. ```ts diff --git a/test/AstAnalyser.spec.js b/test/AstAnalyser.spec.js index 6736cc8..0c611e2 100644 --- a/test/AstAnalyser.spec.js +++ b/test/AstAnalyser.spec.js @@ -266,9 +266,9 @@ describe("AstAnalyser", (t) => { }); }); - it("should instantiate with correct default ASTOptions", () => { + it("should instantiate with correct default options", () => { const analyser = new AstAnalyser(); - assert(analyser.parser instanceof JsSourceParser || typeof analyser.parser.customParser === "object"); + assert.ok(analyser.parser instanceof JsSourceParser); assert.deepStrictEqual(analyser.probesOptions.customProbes, []); assert.strictEqual(analyser.probesOptions.skipDefaultProbes, false); }); From 08ea37027f0cdbd6992698dca26d047ea3d90bfc Mon Sep 17 00:00:00 2001 From: tchapacan Date: Tue, 26 Mar 2024 08:55:40 +0100 Subject: [PATCH 9/9] update readme customProbes --- README.md | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 25e0c10..75607b8 100644 --- a/README.md +++ b/README.md @@ -145,8 +145,9 @@ Below a basic probe that detect a string assignation to `danger`: export const customProbes = [ { name: "customProbeUnsafeDanger", - validateNode: (node, sourceFile) => [node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger"] - , + validateNode: (node, sourceFile) => [ + node.type === "VariableDeclaration" && node.declarations[0].init.value === "danger" + ], main: (node, options) => { const { sourceFile, data: calleeName } = options; if (node.declarations[0].init.value === "danger") { @@ -197,11 +198,12 @@ Result: Congrats, you have created your first custom probe! 🎉 +> [!TIP] > Check the types in [index.d.ts](index.d.ts) and [types/api.d.ts](types/api.d.ts) for more details about the `options` -> + ## API
-declare function runASTAnalysis(str: string, options?: RuntimeOptions & AstAnalyserOptions): Report +runASTAnalysis(str: string, options?: RuntimeOptions & AstAnalyserOptions): Report ```ts interface RuntimeOptions { @@ -234,7 +236,7 @@ interface Report {
-declare function runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions & AstAnalyserOptions): Promise< ReportOnFile > +runASTAnalysisOnFile(pathToFile: string, options?: RuntimeFileOptions & AstAnalyserOptions): Promise< ReportOnFile > ```ts interface RuntimeFileOptions {