Merge pull request #626 from NodeSecure/feat/trace-class-instance-methods

feat(js-x-ray): improve unsafe-vm-context

Author: clemgbld

.changeset/yummy-words-pay.md

@@ -0,0 +1,5 @@
+---
+"@nodesecure/js-x-ray": minor
+---
+
+feat(js-x-ray): improve unsafe-vm-context

docs/unsafe-vm-context.md

@@ -6,15 +6,31 @@
 
 ## Introduction
 
-Detects potentially dangerous use of `vm.runInNewContext()` from the vm module. Despite appearing to provide an isolated execution environment, this API does not constitute a real security sandbox and should be considered as dangerous as a command injection vulnerability. It only separates JavaScript global scopes while sharing the same underlying V8 heap. Any code executed through it can escape the context via prototype chain traversal and gain full access to the host process.
+Detects potentially dangerous use of `vm.runInNewContext()` and `(vm.Script(code,options)).runInContext` from the vm module. Despite appearing to provide an isolated execution environment, this API does not constitute a real security sandbox and should be considered as dangerous as a command injection vulnerability. It only separates JavaScript global scopes while sharing the same underlying V8 heap. Any code executed through it can escape the context via prototype chain traversal and gain full access to the host process.
 
 ## Example
 
 ```js
 import vm from "vm";
 
-const userInput = req.body.expression;
-const result = vm.runInNewContext(userInput, { x: 10, y: 20 });
+// command injection
+
+code = 'var x = this.constructor.constructor("return process.mainModule.require(\'child_process\').execSync(\'cat /etc/passwd\',{encoding:'utf-8'})")()';
+
+const context = {y : 1}
+vm.runInNewContext(code,context);
+console.log(context.x);
+
+// environment variables leak
+
+code = 'var x = this.constructor.constructor("return process.env")()';
+
+const context = {y : 1}
+
+const script = new vm.Script(code);
+
+vm.runInContext(vm.createContext(context));
+console.log(context.x);
 ```
 
 ## References

workspaces/js-x-ray/README.md

@@ -234,7 +234,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-vm-context](https://github.com/NodeSecure/js-x-ray/blob/master/docs/unsafe-vm-context.md) ⚠️ | No | Usage of dangerous vm.runInNewContext |
+| [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
 

workspaces/js-x-ray/docs/VariableTracer.md

@@ -296,6 +296,19 @@ tracer.on(VariableTracer.AssignmentEvent, (payload) => {
 });
 ```
 
+### ReturnValueEvent
+
+Emitted when a traced function's return value is assigned.
+
+```js
+tracer.on(VariableTracer.ReturnValueEvent, (payload) => {
+
+  console.log(`${payload.name} assigned to ${payload.id}`);
+  console.log(`Location:`, payload.location);
+});
+```
+
+
 **Payload:** `AssignmentEventPayload`
 
 ### ImportEvent

workspaces/js-x-ray/src/AstAnalyser.ts

@@ -21,6 +21,7 @@ import {
   PipelineRunner,
   type Pipeline
 } from "./pipelines/index.ts";
+import { Inline } from "./pipelines/inline.ts";
 import { ProbeRunner, type Probe } from "./ProbeRunner.ts";
 import {
   SourceFile,
@@ -163,8 +164,7 @@ export class AstAnalyser extends EventEmitter<AstAnalyserEvents> {
       collectables = [],
       sensitivity = "conservative"
     } = options;
-
-    this.#pipelineRunner = new PipelineRunner(pipelines);
+    this.#pipelineRunner = new PipelineRunner([...pipelines, new Inline()]);
     this.#collectableSetRegistry = new CollectableSetRegistry(
       collectables ?? []
     );

workspaces/js-x-ray/src/Inlined.ts

@@ -0,0 +1,125 @@
+// Import Third-party Dependencies
+import type { ESTree } from "meriyah";
+
+// Import Internal Dependencies
+import { VirtualVariableIdentifier } from "./VirtualVariableIdentifier.ts";
+
+export interface SplitResult {
+  /**
+   * A virtual variable name that replaces the target
+   */
+  virtualIdentifier: string;
+  /**
+   * Virtual variable declaration:
+   * require: const __virtual_require_0__ = require("xxx")
+   * new: const __virtual_new_0__ = new Foo("xxx")
+   * Can be walked with standard ESTree walkers.
+   */
+  virtualDeclaration: ESTree.VariableDeclaration;
+  /**
+   * The rebuilt expression with require() replaced by the virtual identifier.
+   * For `require("x").spawn("y")`, this would be `__virtual_require_0__.spawn("y")`
+   * For `(new Foo()).bar()`, this would be `__virtual_new_0__.bar()`
+   * Can be walked with standard ESTree walkers.
+   */
+  rebuildExpression: ESTree.Node | null;
+}
+
+export class Inlined {
+  static buildSplitResult(
+    node: ESTree.Node,
+    target: ESTree.Expression,
+    identifier: string
+  ): SplitResult {
+    const virtualIdentifier = VirtualVariableIdentifier.generate(
+      identifier,
+      node.loc
+    );
+
+    return {
+      virtualIdentifier,
+      virtualDeclaration: {
+        type: "VariableDeclaration",
+        kind: "const",
+        declarations: [
+          {
+            type: "VariableDeclarator",
+            id: {
+              type: "Identifier",
+              name: virtualIdentifier
+            },
+            init: target
+          }
+        ]
+
+      },
+      rebuildExpression: Inlined.#rebuildWithVirtualIdentifier(
+        node,
+        target,
+        virtualIdentifier
+      )
+    };
+  }
+
+  static #rebuildWithVirtualIdentifier(
+    node: ESTree.Node,
+    target: ESTree.Node,
+    virtualIdentifier: string
+  ): ESTree.Node | null {
+    if (node === target) {
+      return null;
+    }
+
+    const virtualId: ESTree.Identifier = {
+      type: "Identifier",
+      name: virtualIdentifier
+    };
+
+    return Inlined.#cloneAndReplace(
+      node,
+      target,
+      virtualId
+    );
+  }
+
+  static #cloneAndReplace(
+    node: ESTree.Node,
+    target: ESTree.Node,
+    replacement: ESTree.Identifier
+  ): ESTree.Node {
+    if (node === target) {
+      return replacement;
+    }
+
+    if (node.type === "CallExpression") {
+      const callee = Inlined.#cloneAndReplace(
+        node.callee,
+        target,
+        replacement
+      ) as ESTree.Expression;
+
+      const args = node.arguments.map(
+        (arg) => Inlined.#cloneAndReplace(arg, target, replacement)
+      ) as ESTree.Expression[];
+
+      return {
+        ...node,
+        callee,
+        arguments: args
+      };
+    }
+
+    if (node.type === "MemberExpression") {
+      return {
+        ...node,
+        object: Inlined.#cloneAndReplace(
+          node.object,
+          target,
+          replacement
+        ) as ESTree.Expression
+      };
+    }
+
+    return node;
+  }
+}

workspaces/js-x-ray/src/InlinedNew.ts

@@ -0,0 +1,39 @@
+// Import Third-party Dependencies
+import type { ESTree } from "meriyah";
+
+// Import Internal Dependencies
+import { Inlined, type SplitResult } from "./Inlined.ts";
+
+export class InlinedNew {
+  static split(node: ESTree.Node): SplitResult | null {
+    if (node.type !== "CallExpression" && node.type !== "MemberExpression") {
+      return null;
+    }
+    const newExpression = InlinedNew.#findNewCall(node);
+    if (!newExpression) {
+      return null;
+    }
+
+    return Inlined.buildSplitResult(node, newExpression, "new");
+  }
+
+  static #findNewCall(
+    node: ESTree.Node
+  ): ESTree.NewExpression | null {
+    if (node.type === "CallExpression") {
+      const callee = node.callee;
+
+      return InlinedNew.#findNewCall(callee);
+    }
+
+    if (node.type === "MemberExpression") {
+      return InlinedNew.#findNewCall(node.object);
+    }
+
+    if (node.type === "NewExpression") {
+      return node;
+    }
+
+    return null;
+  }
+}

workspaces/js-x-ray/src/ProbeRunner.ts

@@ -347,5 +347,6 @@ export class ProbeRunner {
       probe.finalize?.(this.#getProbeContext(probe));
       probe.context = probe[kProbeOriginalContext];
     }
+    this.sourceFile.tracer.removeAllListeners();
   }
 }

workspaces/js-x-ray/src/VariableTracer.ts

@@ -116,13 +116,19 @@ export interface TracedIdentifierReport {
   assignmentMemory: AssignmentMemory[];
 }
 
-export interface AssignmentEventPayload {
+interface Payload {
   name: string;
   identifierOrMemberExpr: string;
   id: string;
   location: ESTree.SourceLocation | null | undefined;
 }
 
+export interface AssignmentEventPayload extends Payload { }
+
+export interface ReturnValueEventPayload extends Payload {
+  arguments: ESTree.Expression[];
+}
+
 export interface ImportEventPayload {
   moduleName: string;
   value: string;
@@ -137,6 +143,7 @@ export interface LiteralIdentifier {
 export class VariableTracer extends EventEmitter {
   static AssignmentEvent = Symbol("AssignmentEvent");
   static ImportEvent = Symbol("ImportEvent");
+  static ReturnValueEvent = Symbol("ReturnValueEvent");
 
   // PUBLIC PROPERTIES
   literalIdentifiers = new Map<string, LiteralIdentifier>();
@@ -517,6 +524,7 @@ export class VariableTracer extends EventEmitter {
        * const g = eval("this");
        * const g = Function("return this")();
        */
+      case "NewExpression":
       case "CallExpression": {
         const fullIdentifierPath = getCallExpressionIdentifier(childNode);
         if (fullIdentifierPath === null) {
@@ -532,10 +540,21 @@ export class VariableTracer extends EventEmitter {
             type: "ReturnValueAssignment",
             name: id.name
           });
+          const returnValueEventPayload = {
+            name: tracedVariant.name,
+            identifierOrMemberExpr: tracedVariant.identifierOrMemberExpr,
+            id: id.name,
+            location: id.loc,
+            arguments: childNode.arguments
+          };
+          this.emit(VariableTracer.ReturnValueEvent, returnValueEventPayload);
           if (tracedVariant.followConsecutiveAssignment) {
             this.#assignedReturnValueToTraced.set(id.name, tracedFullIdentifierName);
           }
         }
+        if (childNode.type !== "CallExpression") {
+          break;
+        }
         // const id = Function.prototype.call.call(require, require, "http");
         if (this.#neutralCallable.has(identifierName) || isEvilIdentifierPath(fullIdentifierPath)) {
           // TODO: make sure we are walking on a require CallExpr here ?

workspaces/js-x-ray/src/estree/functions/getCallExpressionIdentifier.ts

@@ -24,7 +24,7 @@ export function getCallExpressionIdentifier(
   node: ESTree.Node,
   options: GetCallExpressionIdentifierOptions = {}
 ): string | null {
-  if (node.type !== "CallExpression") {
+  if (node.type !== "CallExpression" && node.type !== "NewExpression") {
     return null;
   }
   const {