fix(unsafe-import): warning on unsafe-import using eval/require

.gitignore

@@ -106,3 +106,4 @@ dist
 temp.js
 temp/
 .vscode/
+.idea/

src/probes/isRequire.js

@@ -11,10 +11,7 @@ import {
   getCallExpressionArguments
 } from "@nodesecure/estree-ast-utils";
 
-// Import Internal Dependencies
-import { ProbeSignals } from "../ProbeRunner.js";
-
-function validateNode(node, { tracer }) {
+function validateNodeRequire(node, { tracer }) {
   const id = getCallExpressionIdentifier(node);
   if (id === null) {
     return [false];
@@ -28,14 +25,35 @@ function validateNode(node, { tracer }) {
   ];
 }
 
+function validateNodeEvalRequire(node, options) {
+  const id = getCallExpressionIdentifier(node);
+  const argument = getCallExpressionArguments(node);
+  if (id === null) {
+    return [false];
+  }
+
+  return [
+    id === "eval" && argument[0] === "require",
+    options?.identifiersName[0]?.name ?? void 0
+  ];
+}
+
+
 function main(node, options) {
-  const { analysis } = options;
+  const { analysis, data: calleeName } = options;
   const { tracer } = analysis;
 
   if (node.arguments.length === 0) {
     return;
   }
   const arg = node.arguments.at(0);
+  const id = getCallExpressionIdentifier(node);
+
+
+  if (arg.value === "require" && id === "eval") {
+    analysis.addWarning("unsafe-import", calleeName, node.loc);
+    analysis.addDependency(calleeName, node.loc, true);
+  }
 
   switch (arg.type) {
     // const foo = "http"; require(foo);
@@ -53,7 +71,9 @@ function main(node, options) {
 
     // require("http")
     case "Literal":
-      analysis.addDependency(arg.value, node.loc);
+      if (arg.value !== "require") {
+        analysis.addDependency(arg.value, node.loc);
+      }
       break;
 
     // require(["ht", "tp"])
@@ -99,7 +119,7 @@ function main(node, options) {
       analysis.addWarning("unsafe-import", null, node.loc);
 
       // We skip walking the tree to avoid anymore warnings...
-      return ProbeSignals.Skip;
+      return Symbol.for("skipWalk");
     }
 
     default:
@@ -163,8 +183,5 @@ function walkRequireCallExpression(nodeToWalk, tracer) {
 
 export default {
   name: "isRequire",
-  validateNode,
-  main,
-  breakOnMatch: true,
-  breakGroup: "import"
+  validateNode: [validateNodeRequire, validateNodeEvalRequire], main, breakOnMatch: true, breakGroup: "import"
 };

src/probes/isUnsafeCallee.js

@@ -1,6 +1,5 @@
 // Import Internal Dependencies
 import { isUnsafeCallee } from "../utils.js";
-import { ProbeSignals } from "../ProbeRunner.js";
 
 /**
  * @description Detect unsafe statement
@@ -16,13 +15,9 @@ function main(node, options) {
   const { analysis, data: calleeName } = options;
 
   analysis.addWarning("unsafe-stmt", calleeName, node.loc);
-
-  return ProbeSignals.Skip;
 }
 
 export default {
   name: "isUnsafeCallee",
-  validateNode,
-  main,
-  breakOnMatch: false
+  validateNode, main, breakOnMatch: false
 };

src/utils.js

@@ -13,7 +13,8 @@ export function isUnsafeCallee(node) {
   // For Function we are looking for this: `Function("...")();`
   // A double CallExpression
   return [
-    identifier === "eval" || (identifier === "Function" && node.callee.type === "CallExpression"),
+    (identifier === "eval" && node.callee.type === "Identifier") ||
+    (identifier === "Function" && node.callee.type === "CallExpression"),
     identifier
   ];
 }

test/issues/179-UnsafeEvalRequire.spec.js

@@ -0,0 +1,27 @@
+// Import Node.js Dependencies
+import { test } from "node:test";
+import assert from "node:assert";
+
+// Import Internal Dependencies
+import { runASTAnalysis } from "../../index.js";
+
+/**
+ * @see https://github.com/NodeSecure/js-x-ray/issues/179
+ */
+// CONSTANTS
+const kIncriminedCodeSample = `const stream = eval('require')('stream');`;
+const kWarningUnsafeImport = "unsafe-import";
+const kWarningUnsafeStatement = "unsafe-stmt";
+
+test("should detect unsafe-import and unsafe-statement", () => {
+  const sastAnalysis = runASTAnalysis(kIncriminedCodeSample);
+
+  assert.equal(sastAnalysis.warnings.at(0).value, "eval");
+  assert.equal(sastAnalysis.warnings.at(0).kind, kWarningUnsafeStatement);
+  assert.equal(sastAnalysis.warnings.at(1).value, "stream");
+  assert.equal(sastAnalysis.warnings.at(1).kind, kWarningUnsafeImport);
+  assert.equal(sastAnalysis.warnings.length, 2);
+  assert.equal(sastAnalysis.dependencies.has("stream"), true);
+  assert.equal(sastAnalysis.dependencies.get("stream").unsafe, true);
+  assert.equal(sastAnalysis.dependencies.size, 1);
+});