From 0e19cbe791cafc48b3d19ac1685cdab67a01477d Mon Sep 17 00:00:00 2001 From: "Raulo Erwan." Date: Sun, 21 Jun 2026 01:14:17 +0200 Subject: [PATCH 1/2] feat(probe): unsafe prehash digest with null-byte encoding --- .changeset/bees-truly-shine.md | 5 + docs/unsafe-prehash.md | 99 ++++ workspaces/js-x-ray/README.md | 5 +- workspaces/js-x-ray/src/ProbeRunner.ts | 4 +- .../js-x-ray/src/probes/isUnsafePrehash.ts | 258 +++++++++++ workspaces/js-x-ray/src/warnings.ts | 8 +- .../test/probes/isUnsafePrehash.spec.ts | 438 ++++++++++++++++++ 7 files changed, 814 insertions(+), 3 deletions(-) create mode 100644 .changeset/bees-truly-shine.md create mode 100644 docs/unsafe-prehash.md create mode 100644 workspaces/js-x-ray/src/probes/isUnsafePrehash.ts create mode 100644 workspaces/js-x-ray/test/probes/isUnsafePrehash.spec.ts diff --git a/.changeset/bees-truly-shine.md b/.changeset/bees-truly-shine.md new file mode 100644 index 00000000..34602bb4 --- /dev/null +++ b/.changeset/bees-truly-shine.md @@ -0,0 +1,5 @@ +--- +"@nodesecure/js-x-ray": minor +--- + +feat(js-x-ray): add unsafe-prehash warning diff --git a/docs/unsafe-prehash.md b/docs/unsafe-prehash.md new file mode 100644 index 00000000..b5ebba23 --- /dev/null +++ b/docs/unsafe-prehash.md @@ -0,0 +1,99 @@ +# Unsafe prehash + +| Code | Severity | i18n | Experimental | +| --- | --- | --- | :-: | +| unsafe-prehash | `Warning` | `sast_warnings.unsafe_prehash` | :white_check_mark: | + +## Introduction + +Detect **unsafe password pre-hashing** before passing the result to [`bcryptjs`](https://www.npmjs.com/package/bcryptjs)'s `bcrypt.hash()` / `bcrypt.hashSync()`. + +Only `bcryptjs` is covered, not the `bcrypt` package. It gets roughly 2x `bcrypt`'s weekly npm downloads. Extending this probe to also cover `bcrypt` is a separate work. + +Pre-hashing a password by passing the raw digest `Buffer` directly can be dangerous: bcrypt treats its input as a null-terminated string, so a digest containing a `0x00` byte gets silently truncated at that byte, increasing the chance of hash collisions. + +This probe flags `bcrypt.hash()` / `bcrypt.hashSync()` calls (from `bcryptjs`) whose first argument is a `digest(encoding)` or `.digest().toString(encoding)` call that does not use a null-byte-free encoding (base64, base64url or hex). + +The probe also follows the common pattern of computing the digest first and passing it to bcrypt by reference. + +## How detection works + +`bcryptjs` is tracked through the codebase's shared `VariableTracer`, so all the common import forms are recognized: + +```js +import bcrypt from "bcryptjs"; +import * as bcrypt from "bcryptjs"; +import { hash } from "bcryptjs"; +// except this one: not recognized +import { hash as bcryptHash } from "bcryptjs"; +``` + + +## Example + +```js +import bcrypt from "bcryptjs"; +import crypto from "crypto"; + +// unsafe: raw digest Buffer may contain a null byte +bcrypt.hash(crypto.createHash("sha256").update(password).digest(), salt, callback); + +// unsafe: same issue, written as digest().toString(...) +bcrypt.hash(crypto.createHash("sha256").update(password).digest().toString("binary"), salt, callback); + +// unsafe: same issue, with the digest stored in a variable first +const prehashed = crypto.createHash("sha256").update(password).digest(); +bcrypt.hash(prehashed, salt, callback); + +// safe: base64 output never contains a null byte +bcrypt.hash(crypto.createHash("sha256").update(password).digest("base64"), salt, callback); + +// safe: digest() already returned a safely-encoded hex string +bcrypt.hash(crypto.createHash("sha256").update(password).digest("hex").toString("binary"), salt, callback); +``` + +## Limitations + +This probe is purely name-based and has no lexical scope resolution. Several consequences: + +- **The `bcrypt` package is not detected at all.** + +- **Renamed named imports aren't recognized.** This is a separate, `VariableTracer` limitation. + + ```js + // not detected: aliased via "as", the local binding "bcryptHash" isn't tracked + import { hash as bcryptHash } from "bcryptjs"; + bcryptHash(crypto.createHash("sha256").update(password).digest(), salt, callback); + ``` + +- **Variable indirection only works one level deep, in source order.** The variable must be declared with `const`/`let`/`var` and an initializer earlier in the same file. +Reassignment, destructuring, function parameters, or a digest returned from a helper function are not tracked: + + ```js + // not detected: the digest only reaches bcrypt through a function parameter + function hashPassword(prehashed) { + bcrypt.hash(prehashed, salt, callback); + } + hashPassword(crypto.createHash("sha256").update(password).digest()); + ``` + +- **Matching is by identifier name only, not by binding/scope, with two safeguards.** A name is excluded from matching if it's ever used as a function/arrow parameter anywhere in the file, *or* if it's declared by more than one `VariableDeclarator` anywhere in the file (a sign that two unrelated bindings happen to share a name). Together these cover the common collision shapes: + + ```js + function foo() { + const prehashed = crypto.createHash("sha256").update(password).digest(); // unsafe, name "prehashed" is remembered + } + + function bar(prehashed) { // excluded: "prehashed" is used as a parameter name elsewhere + bcrypt.hash(prehashed, salt, callback); // not flagged + } + + function baz() { + const prehashed = someUnrelatedSafeValue(); // excluded: "prehashed" is declared more than once in the file + bcrypt.hash(prehashed, salt, callback); // not flagged + } + ``` + +## References + +- [OWASP Password Storage Cheat Sheet — Pre-Hashing Passwords with bcrypt](https://cheatsheetseries.owasp.org/cheatsheets/Password_Storage_Cheat_Sheet.html#pre-hashing-passwords-with-bcrypt) diff --git a/workspaces/js-x-ray/README.md b/workspaces/js-x-ray/README.md index b462e853..769348b0 100644 --- a/workspaces/js-x-ray/README.md +++ b/workspaces/js-x-ray/README.md @@ -124,7 +124,8 @@ Alternatively, you can use `EntryFilesAnalyser` directly for multi-file analysis type OptionalWarningName = | "synchronous-io" | "log-usage" - | "weak-scrypt"; + | "weak-scrypt" + | "unsafe-prehash"; type WarningName = | "parsing-error" @@ -187,6 +188,7 @@ The following warnings are optional: - `synchronous-io` - Detects synchronous I/O operations that could impact performance - `log-usage` - Tracks usage of logging functions (console.log, logger.info, etc.) - `weak-scrypt` - Detects weak scrypt parameters (low cost, short or hardcoded salt) +- `unsafe-prehash` - Detects password pre-hashing fed into bcrypt without a safe (base64/hex) digest encoding ### Internationalization (i18n) @@ -234,6 +236,7 @@ Click on the warning **name** for detailed documentation and examples. | [monkey-patch](https://github.com/NodeSecure/js-x-ray/blob/master/docs/monkey-patch.md) | No | Modification of built-in JavaScript prototype properties | | [prototype-pollution](https://github.com/NodeSecure/js-x-ray/blob/master/docs/prototype-pollution.md) | No | Detected use of `__proto__` to pollute object prototypes | | [weak-scrypt](https://github.com/NodeSecure/js-x-ray/blob/master/docs/weak-scrypt.md) ⚠️ | **Yes** | Usage of weak scrypt parameters (low cost, short or hardcoded salt) | +| [unsafe-prehash](https://github.com/NodeSecure/js-x-ray/blob/master/docs/unsafe-prehash.md) ⚠️ | **Yes** | Password pre-hashed and fed into bcrypt without a safe digest encoding | | [unsafe-vm-context](https://github.com/NodeSecure/js-x-ray/blob/master/docs/unsafe-vm-context.md) ⚠️ | No | Usage of dangerous vm.runInNewContext and (vm.Script(code,options)).runInContext | #### Information Severity diff --git a/workspaces/js-x-ray/src/ProbeRunner.ts b/workspaces/js-x-ray/src/ProbeRunner.ts index d63560ed..43c2f703 100644 --- a/workspaces/js-x-ray/src/ProbeRunner.ts +++ b/workspaces/js-x-ray/src/ProbeRunner.ts @@ -27,6 +27,7 @@ import isMonkeyPatch from "./probes/isMonkeyPatch.ts"; import isRandom from "./probes/isRandom.ts"; import isPrototypePollution from "./probes/isPrototypePollution.ts"; import isWeakScrypt from "./probes/isWeakScrypt.ts"; +import isUnsafePrehash from "./probes/isUnsafePrehash.ts"; import type { TracedIdentifierReport } from "./VariableTracer.ts"; import type { SourceFile } from "./SourceFile.ts"; @@ -128,7 +129,8 @@ export class ProbeRunner { "synchronous-io": isSyncIO, "log-usage": logUsage, "insecure-random": isRandom, - "weak-scrypt": isWeakScrypt + "weak-scrypt": isWeakScrypt, + "unsafe-prehash": isUnsafePrehash }; constructor( diff --git a/workspaces/js-x-ray/src/probes/isUnsafePrehash.ts b/workspaces/js-x-ray/src/probes/isUnsafePrehash.ts new file mode 100644 index 00000000..ee1f4994 --- /dev/null +++ b/workspaces/js-x-ray/src/probes/isUnsafePrehash.ts @@ -0,0 +1,258 @@ +// Import Third-party Dependencies +import type { ESTree } from "meriyah"; + +// Import Internal Dependencies +import type { ProbeContext, ProbeMainContext } from "../ProbeRunner.ts"; +import { CALL_EXPRESSION_DATA } from "../contants.ts"; +import { isCallExpression, isLiteral } from "../estree/types.ts"; +import { getVariableDeclarationIdentifiers } from "../estree/index.ts"; +import { generateWarning } from "../warnings.ts"; + +const kModuleName = "bcryptjs"; +const kTracedFunctions = new Set(["bcryptjs.hash", "bcryptjs.hashSync"]); + +// Set of variable names known to hold an unsafely-encoded digest +const kUnsafeDigestVariables = Symbol("unsafeDigestVariables"); + +// Set of names that can't be trusted by name alone +const kAmbiguousVariableNames = Symbol("ambiguousVariableNames"); + +// Plain variable names declared via a VariableDeclarator regardless of its initializer +const kSeenVariableNames = Symbol("seenVariableNames"); + +interface UnsafePrehashContext { + [kUnsafeDigestVariables]?: Set; + [kAmbiguousVariableNames]?: Set; + [kSeenVariableNames]?: Set; +} + +type UnsafePrehashContextSetKey = + | typeof kUnsafeDigestVariables + | typeof kAmbiguousVariableNames + | typeof kSeenVariableNames; + +function getContextSet( + ctx: ProbeContext, + key: UnsafePrehashContextSetKey +): Set { + return ctx.context![key]!; +} + +/** + * Digest encodings that produce ASCII-only output, avoiding the null-byte truncation issue + */ +const kSafeDigestEncodings = new Set(["base64", "base64url", "hex"]); + +type MemberCallExpression = ESTree.CallExpression & { callee: ESTree.MemberExpression; }; + +function getMemberCallExpression( + node: ESTree.Node | null | undefined, + methodName: string +): MemberCallExpression | null { + if ( + isCallExpression(node) && + node.callee.type === "MemberExpression" && + !node.callee.computed && + node.callee.property.type === "Identifier" && + node.callee.property.name === methodName + ) { + return node; + } + + return null; +} + +/** + * Resolves both `x.digest(encoding)` and `x.digest().toString(encoding)` + */ +function resolveDigestEncodingArguments( + hashNode: ESTree.Node | null | undefined +): ESTree.Node[] | null { + const digestCall = getMemberCallExpression(hashNode, "digest"); + if (digestCall) { + return digestCall.arguments; + } + + const toStringCall = getMemberCallExpression(hashNode, "toString"); + if (toStringCall) { + const innerDigestCall = getMemberCallExpression(toStringCall.callee.object, "digest"); + if (innerDigestCall) { + return innerDigestCall.arguments.length === 0 + ? toStringCall.arguments + : innerDigestCall.arguments; + } + } + + return null; +} + +function getParamNames( + params: ESTree.Node[] +): string[] { + const names: string[] = []; + + for (const param of params) { + for (const { assignmentId } of getVariableDeclarationIdentifiers(param)) { + names.push(assignmentId.name); + } + } + + return names; +} + +function hasUnsafeDigestEncoding( + hashNode: ESTree.Node | null | undefined +): boolean { + const encodingArgs = resolveDigestEncodingArguments(hashNode); + if (encodingArgs === null) { + return false; + } + + const encodingArg = encodingArgs.at(0); + + return !(isLiteral(encodingArg) && kSafeDigestEncodings.has(encodingArg.value)); +} + +type FunctionParamNames = string[]; +type VariableIdentifierName = string; + +type NodeValidationResult = + [false] | + [true] | + [true, FunctionParamNames] | + [true, { name: VariableIdentifierName; isUnsafe: boolean; }]; + +function validateNode( + node: ESTree.Node, + ctx: ProbeContext +): NodeValidationResult { + const { tracer } = ctx.sourceFile; + + if (!tracer.importedModules.has(kModuleName) || !tracer.importedModules.has("crypto")) { + return [false]; + } + + if (node.type === "VariableDeclarator") { + if (node.id.type !== "Identifier") { + return [false]; + } + + ctx.setEntryPoint("trackVariableDeclarator"); + + return [true, { + name: node.id.name, + isUnsafe: hasUnsafeDigestEncoding(node.init) + }]; + } + + if ( + node.type === "FunctionDeclaration" || + node.type === "FunctionExpression" || + node.type === "ArrowFunctionExpression" + ) { + const paramNames = getParamNames(node.params); + if (paramNames.length === 0) { + return [false]; + } + + ctx.setEntryPoint("markAmbiguousParams"); + + return [true, paramNames]; + } + + return [ + kTracedFunctions.has(ctx.context![CALL_EXPRESSION_DATA]?.identifierOrMemberExpr) + ]; +} + +function initialize(ctx: ProbeContext) { + const { tracer } = ctx.sourceFile; + + ctx.context![kUnsafeDigestVariables] = new Set(); + ctx.context![kAmbiguousVariableNames] = new Set(); + ctx.context![kSeenVariableNames] = new Set(); + + for (const identifierOrMemberExpr of kTracedFunctions) { + tracer.trace(identifierOrMemberExpr, { + followConsecutiveAssignment: true, + moduleName: kModuleName + }); + } +} + +function trackVariableDeclarator( + _node: ESTree.VariableDeclarator, + ctx: ProbeMainContext +) { + const { name, isUnsafe } = ctx.data as { name: string; isUnsafe: boolean; }; + const seenVariableNames = getContextSet(ctx, kSeenVariableNames); + + if (seenVariableNames.has(name)) { + getContextSet(ctx, kAmbiguousVariableNames).add(name); + } + else { + seenVariableNames.add(name); + } + + if (isUnsafe) { + getContextSet(ctx, kUnsafeDigestVariables).add(name); + } +} + +function markAmbiguousParams( + _node: ESTree.Node, + ctx: ProbeMainContext +) { + const ambiguousVariableNames = getContextSet(ctx, kAmbiguousVariableNames); + for (const name of ctx.data as string[]) { + ambiguousVariableNames.add(name); + } +} + +function bcryptHashCall( + bcryptNode: ESTree.CallExpression, + ctx: ProbeMainContext +) { + const { sourceFile } = ctx; + const hashArgument = bcryptNode.arguments.at(0); + + let isUnsafe: boolean; + if (hashArgument?.type === "Identifier") { + const isAmbiguous = getContextSet(ctx, kAmbiguousVariableNames).has(hashArgument.name); + const isDigestVariable = getContextSet(ctx, kUnsafeDigestVariables).has(hashArgument.name); + + isUnsafe = !isAmbiguous && isDigestVariable; + } + else { + isUnsafe = hasUnsafeDigestEncoding(hashArgument); + } + + if (isUnsafe) { + sourceFile.warnings.push( + generateWarning("unsafe-prehash", { + value: null, + location: bcryptNode.loc + }) + ); + } +} + +export default { + name: "isUnsafePrehash", + nodeTypes: [ + "CallExpression", + "VariableDeclarator", + "FunctionDeclaration", + "FunctionExpression", + "ArrowFunctionExpression" + ], + validateNode, + main: { + default: bcryptHashCall, + trackVariableDeclarator, + markAmbiguousParams + }, + initialize, + breakOnMatch: false, + context: {} +}; diff --git a/workspaces/js-x-ray/src/warnings.ts b/workspaces/js-x-ray/src/warnings.ts index 3a82ce90..887e56a5 100644 --- a/workspaces/js-x-ray/src/warnings.ts +++ b/workspaces/js-x-ray/src/warnings.ts @@ -13,7 +13,8 @@ export type OptionalWarningName = | "synchronous-io" | "log-usage" | "insecure-random" - | "weak-scrypt"; + | "weak-scrypt" + | "unsafe-prehash"; export type WarningName = | "parsing-error" @@ -154,6 +155,11 @@ export const warnings = Object.freeze({ severity: "Warning", experimental: true }, + "unsafe-prehash": { + i18n: "sast_warnings.unsafe_prehash", + severity: "Warning", + experimental: true + }, "unsafe-vm-context": { i18n: "sast_warnings.unsafe_vm_context", severity: "Warning", diff --git a/workspaces/js-x-ray/test/probes/isUnsafePrehash.spec.ts b/workspaces/js-x-ray/test/probes/isUnsafePrehash.spec.ts new file mode 100644 index 00000000..78b0f1c2 --- /dev/null +++ b/workspaces/js-x-ray/test/probes/isUnsafePrehash.spec.ts @@ -0,0 +1,438 @@ +// Import Node.js Dependencies +import assert from "node:assert"; +import { describe, it } from "node:test"; + +// Import Internal Dependencies +import { AstAnalyser } from "../../src/index.ts"; + +describe("isUnsafePrehash probe", () => { + describe("unsafe-prehash", () => { + it("should warn when a raw digest() (no encoding) is passed to bcryptjs.hash", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when a raw digest() (no encoding) is passed to bcryptjs.hashSync", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + const hash = bcrypt.hashSync(crypto.createHash('sha256').update(password).digest(), salt); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when digest() uses an unsafe encoding (binary/latin1)", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest('binary'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn for createHmac digest chains too", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHmac('sha384', pepper).update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when digest().toString() (no encoding) is used", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest().toString(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when digest().toString('binary') is used", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest().toString('binary'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when digest('binary').toString('hex') is used (outer toString is a no-op)", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest('binary').toString('hex'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when bcryptjs is imported as a namespace", () => { + const code = ` + import * as bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when bcryptjs.hash is imported as a named import", () => { + const code = ` + import { hash } from 'bcryptjs'; + import crypto from 'crypto'; + hash(crypto.createHash('sha256').update(password).digest(), salt, (err, h) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + }); + + describe("variable indirection", () => { + it("should warn when an unsafely pre-hashed variable is passed to bcryptjs.hash", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + const prehashed = crypto.createHash('sha256').update(password).digest(); + bcrypt.hash(prehashed, salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should warn when an unsafely pre-hashed variable (via toString) is passed to bcryptjs.hashSync", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + const prehashed = crypto.createHash('sha256').update(password).digest().toString('binary'); + const hash = bcrypt.hashSync(prehashed, salt); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + + it("should not warn when a safely pre-hashed variable is passed to bcryptjs.hash", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + const prehashed = crypto.createHash('sha256').update(password).digest('base64'); + bcrypt.hash(prehashed, salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when the bcryptjs argument variable is unrelated to any digest", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + const plainValue = getUserInput(); + bcrypt.hash(plainValue, salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when an unrelated function parameter shares its name with an unsafe pre-hashed variable", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + function bar(prehashed) { + bcrypt.hash(prehashed, salt, (err, hash) => {}); + } + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when an unrelated arrow function parameter shares its name with an unsafe pre-hashed variable", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + const bar = (prehashed) => { + bcrypt.hash(prehashed, salt, (err, hash) => {}); + }; + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when an unrelated destructured parameter shares its name with an unsafe pre-hashed variable", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + function foo() { + const prehashed = crypto.createHash('sha256').update(password).digest(); + } + + function bar({ prehashed }) { + bcrypt.hash(prehashed, salt, (err, hash) => {}); + } + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when an unrelated local variable shares its name with an unsafe pre-hashed variable", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + function bar() { + const prehashed = someUnrelatedSafeValue(); + bcrypt.hash(prehashed, salt, (err, hash) => {}); + } + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should still warn for the unsafe declarator itself when its name later collides with an unrelated variable", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + function foo() { + const prehashed = crypto.createHash('sha256').update(password).digest(); + bcrypt.hash(prehashed, salt, (err, hash) => {}); + } + + function bar() { + const prehashed = someUnrelatedSafeValue(); + } + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 1); + assert.strictEqual(outputWarnings[0].kind, "unsafe-prehash"); + }); + }); + + describe("no warning (proper usage)", () => { + it("should not warn when digest() uses base64 encoding", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest('base64'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when digest() uses hex encoding", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest('hex'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when digest().toString('hex') is used", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest().toString('hex'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when digest('hex').toString('binary') is used", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest('hex').toString('binary'), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn for a plain bcryptjs.hash call without pre-hashing", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(password, salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when crypto module is not imported", () => { + const code = ` + import bcrypt from 'bcryptjs'; + const crypto = { createHash() { return { update() { return this; }, digest() {} }; } }; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when bcryptjs module is not imported", () => { + const code = ` + import crypto from 'crypto'; + const bcrypt = { hash() {} }; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + }); + + describe("known limitations", () => { + it("should not warn for the real bcrypt package (only bcryptjs is supported)", () => { + const code = ` + import bcrypt from 'bcrypt'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn for a renamed bcryptjs named import (known VariableTracer limitation)", () => { + const code = ` + import { hash as bcryptHash } from 'bcryptjs'; + import crypto from 'crypto'; + bcryptHash(crypto.createHash('sha256').update(password).digest(), salt, (err, h) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + + it("should not warn when an unsafely pre-hashed digest reaches bcrypt through a function parameter", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + + function hashPassword(prehashed) { + bcrypt.hash(prehashed, salt, callback); + } + hashPassword(crypto.createHash('sha256').update(password).digest()); + `; + const { warnings: outputWarnings } = new AstAnalyser({ + optionalWarnings: ["unsafe-prehash"] + }).analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + }); + + describe("optional warning behavior", () => { + it("should NOT report warnings when unsafe-prehash is not enabled", () => { + const code = ` + import bcrypt from 'bcryptjs'; + import crypto from 'crypto'; + bcrypt.hash(crypto.createHash('sha256').update(password).digest(), salt, (err, hash) => {}); + `; + const { warnings: outputWarnings } = new AstAnalyser().analyse(code); + + assert.strictEqual(outputWarnings.length, 0); + }); + }); +}); From 20a6969e5b9226fdcdb7fffc18a6c39bceb30dfa Mon Sep 17 00:00:00 2001 From: "Raulo Erwan." Date: Mon, 29 Jun 2026 15:12:55 +0200 Subject: [PATCH 2/2] feat(tracer): trace default imports --- workspaces/js-x-ray/src/VariableTracer.ts | 8 +++ .../VariableTracer/VariableTracer.spec.ts | 52 +++++++++++++++++++ 2 files changed, 60 insertions(+) diff --git a/workspaces/js-x-ray/src/VariableTracer.ts b/workspaces/js-x-ray/src/VariableTracer.ts index 95532a16..54f2a7ab 100644 --- a/workspaces/js-x-ray/src/VariableTracer.ts +++ b/workspaces/js-x-ray/src/VariableTracer.ts @@ -399,6 +399,14 @@ export class VariableTracer extends EventEmitter { return; } + // import boo from "crypto"; + if (node.specifiers[0].type === "ImportDefaultSpecifier") { + const importDefaultNode = node.specifiers[0]; + this.#declareNewAssignment(moduleName, importDefaultNode.local); + + return; + } + // import { createHash } from "crypto"; const importSpecifiers = node.specifiers .filter((specifierNode) => specifierNode.type === "ImportSpecifier"); diff --git a/workspaces/js-x-ray/test/VariableTracer/VariableTracer.spec.ts b/workspaces/js-x-ray/test/VariableTracer/VariableTracer.spec.ts index 14b57ab2..0189c51b 100644 --- a/workspaces/js-x-ray/test/VariableTracer/VariableTracer.spec.ts +++ b/workspaces/js-x-ray/test/VariableTracer/VariableTracer.spec.ts @@ -45,6 +45,58 @@ test("it should trace re-assignment from a module import using /promises", () => }); }); +test("it should trace a default import aliased to a different local name", () => { + const helpers = createTracer(false); + helpers.tracer.trace("crypto.createHash", { + followConsecutiveAssignment: true, + moduleName: "crypto" + }); + helpers.walkOnCode(` + import c from "crypto"; + + const h = c.createHash("md5"); + `); + + const result = helpers.tracer.getDataFromIdentifier("c.createHash"); + + assert.deepEqual(result, { + assignmentMemory: [ + { + type: "AliasBinding", + name: "c" + } + ], + identifierOrMemberExpr: "crypto.createHash", + name: "crypto.createHash" + }); +}); + +test("it should trace a namespace import aliased to a different local name", () => { + const helpers = createTracer(false); + helpers.tracer.trace("crypto.createHash", { + followConsecutiveAssignment: true, + moduleName: "crypto" + }); + helpers.walkOnCode(` + import * as c from "crypto"; + + const h = c.createHash("md5"); + `); + + const result = helpers.tracer.getDataFromIdentifier("c.createHash"); + + assert.deepEqual(result, { + assignmentMemory: [ + { + type: "AliasBinding", + name: "c" + } + ], + identifierOrMemberExpr: "crypto.createHash", + name: "crypto.createHash" + }); +}); + test("it should be able to Trace a malicious code with Global, BinaryExpr, Assignments and Hexadecimal", () => { const helpers = createTracer(true); const assignments = helpers.getAssignmentArray();