Merge pull request #18623 from asgerf/js/nest-di

JS: Add support for dependency injection in Nest

Author: asgerf

javascript/ql/lib/semmle/javascript/dataflow/Nodes.qll

@@ -8,6 +8,7 @@ private import javascript
 private import semmle.javascript.dependencies.Dependencies
 private import internal.CallGraphs
 private import semmle.javascript.internal.CachedStages
+private import semmle.javascript.dataflow.internal.PreCallGraphStep
 
 /**
  * A data flow node corresponding to an expression.
@@ -995,6 +996,9 @@ class ClassNode extends DataFlow::SourceNode instanceof ClassNode::Range {
       result.getAstNode().getFile() = this.getAstNode().getFile()
     )
     or
+    t.start() and
+    PreCallGraphStep::classObjectSource(this, result)
+    or
     result = this.getAClassReferenceRec(t)
   }
 
@@ -1044,6 +1048,9 @@ class ClassNode extends DataFlow::SourceNode instanceof ClassNode::Range {
     // Note that this also blocks flows into a property of the receiver,
     // but the `localFieldStep` rule will often compensate for this.
     not result = any(DataFlow::ClassNode cls).getAReceiverNode()
+    or
+    t.start() and
+    PreCallGraphStep::classInstanceSource(this, result)
   }
 
   pragma[noinline]

javascript/ql/lib/semmle/javascript/dataflow/internal/PreCallGraphStep.qll

@@ -44,6 +44,16 @@ class PreCallGraphStep extends Unit {
   ) {
     none()
   }
+
+  /**
+   * Holds if `node` can hold an instance of `cls`.
+   */
+  predicate classInstanceSource(DataFlow::ClassNode cls, DataFlow::Node node) { none() }
+
+  /**
+   * Holds if `node` can hold an reference to the `cls` class itself.
+   */
+  predicate classObjectSource(DataFlow::ClassNode cls, DataFlow::Node node) { none() }
 }
 
 cached
@@ -90,6 +100,22 @@ module PreCallGraphStep {
   ) {
     any(PreCallGraphStep s).loadStoreStep(pred, succ, loadProp, storeProp)
   }
+
+  /**
+   * Holds if `node` can hold an instance of `cls`.
+   */
+  cached
+  predicate classInstanceSource(DataFlow::ClassNode cls, DataFlow::Node node) {
+    any(PreCallGraphStep s).classInstanceSource(cls, node)
+  }
+
+  /**
+   * Holds if `node` can hold an reference to the `cls` class itself.
+   */
+  cached
+  predicate classObjectSource(DataFlow::ClassNode cls, DataFlow::Node node) {
+    any(PreCallGraphStep s).classObjectSource(cls, node)
+  }
 }
 
 /**

javascript/ql/lib/semmle/javascript/frameworks/Nest.qll

@@ -4,6 +4,7 @@
 
 import javascript
 private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
+private import semmle.javascript.dataflow.internal.PreCallGraphStep
 
 /**
  * Provides classes and predicates for reasoning about [Nest](https://nestjs.com/).
@@ -462,4 +463,54 @@ module NestJS {
       result.(DataFlow::FunctionNode).getAParameter() = this
     }
   }
+
+  /**
+   * A value passed in the `providers` array in:
+   * ```js
+   * @Module({ providers: [ ... ] })
+   * class App { ... }
+   * ```
+   */
+  private DataFlow::Node providerTuple() {
+    result =
+      DataFlow::moduleImport("@nestjs/common")
+          .getAPropertyRead("Module")
+          .getACall()
+          .getOptionArgument(0, "providers")
+          .getALocalSource()
+          .(DataFlow::ArrayCreationNode)
+          .getAnElement()
+  }
+
+  private predicate providerPair(DataFlow::Node interface, DataFlow::Node concreteClass) {
+    exists(DataFlow::SourceNode tuple |
+      tuple = providerTuple().getALocalSource() and
+      interface = tuple.getAPropertyWrite("provide").getRhs() and
+      concreteClass = tuple.getAPropertyWrite("useClass").getRhs()
+    )
+  }
+
+  /** Gets the class being referenced at `node` without relying on the call graph. */
+  private DataFlow::ClassNode getClassFromNode(DataFlow::Node node) {
+    result.getAstNode() = node.analyze().getAValue().(AbstractClass).getClass()
+  }
+
+  private predicate providerClassPair(
+    DataFlow::ClassNode interface, DataFlow::ClassNode concreteClass
+  ) {
+    exists(DataFlow::Node interfaceNode, DataFlow::Node concreteClassNode |
+      providerPair(interfaceNode, concreteClassNode) and
+      interface = getClassFromNode(interfaceNode) and
+      concreteClass = getClassFromNode(concreteClassNode)
+    )
+  }
+
+  private class DependencyInjectionStep extends PreCallGraphStep {
+    override predicate classInstanceSource(DataFlow::ClassNode cls, DataFlow::Node node) {
+      exists(DataFlow::ClassNode interfaceClass |
+        node.asExpr().(Parameter).getType().(ClassType).getClass() = interfaceClass.getAstNode() and
+        providerClassPair(interfaceClass, cls)
+      )
+    }
+  }
 }

javascript/ql/src/change-notes/2025-01-30-nest-di.md

@@ -0,0 +1,5 @@
+---
+category: majorAnalysis
+---
+* Improved support for NestJS applications that make use of dependency injection with custom providers.
+  Calls to methods on an injected service should now be resolved properly.

javascript/ql/test/library-tests/frameworks/Nest/global/app.module.ts

@@ -0,0 +1,12 @@
+import { Module } from '@nestjs/common';
+import { Controller } from './validation';
+import { Foo } from './foo.interface';
+import { FooImpl } from './foo.impl';
+
+@Module({
+    controllers: [Controller],
+    providers: [{
+        provide: Foo, useClass: FooImpl
+    }],
+})
+export class AppModule { }

javascript/ql/test/library-tests/frameworks/Nest/global/foo.impl.ts

@@ -0,0 +1,7 @@
+import { Foo } from "./foo.interface";
+
+export class FooImpl extends Foo {
+    fooMethod(x: string) {
+        sink(x); // $ hasValueFlow=x
+    }
+}

javascript/ql/test/library-tests/frameworks/Nest/global/foo.interface.ts

@@ -0,0 +1,3 @@
+export abstract class Foo {
+    abstract fooMethod(x: string): void;
+}

javascript/ql/test/library-tests/frameworks/Nest/global/validation.ts

@@ -1,12 +1,22 @@
 import { Get, Query } from '@nestjs/common';
 import { IsIn } from 'class-validator';
+import { Foo } from './foo.interface';
 
 export class Controller {
+  constructor(
+    private readonly foo: Foo
+  ) { }
+
   @Get()
   route1(@Query('x') validatedObj: Struct, @Query('y') unvalidated: string) {
     if (Math.random()) return unvalidated; // NOT OK
     return validatedObj.key; // OK
   }
+
+  @Get()
+  route2(@Query('x') x: string) {
+    this.foo.fooMethod(x);
+  }
 }
 
 class Struct {

javascript/ql/test/library-tests/frameworks/Nest/test.expected

@@ -1,5 +1,7 @@
+testFailures
 routeHandler
-| global/validation.ts:6:3:9:3 | route1( ...  OK\\n  } |
+| global/validation.ts:11:3:14:3 | route1( ...  OK\\n  } |
+| global/validation.ts:17:3:19:3 | route2( ... x);\\n  } |
 | local/customDecorator.ts:18:3:20:3 | sneaky( ...  OK\\n  } |
 | local/customDecorator.ts:23:3:25:3 | safe(@S ...  OK\\n  } |
 | local/customPipe.ts:20:5:22:5 | sanitiz ... K\\n    } |
@@ -36,8 +38,9 @@ requestInputAccess
 | body | local/routes.ts:40:16:40:19 | body |
 | body | local/routes.ts:66:26:66:29 | file |
 | body | local/routes.ts:71:31:71:35 | files |
-| parameter | global/validation.ts:6:22:6:33 | validatedObj |
-| parameter | global/validation.ts:6:56:6:66 | unvalidated |
+| parameter | global/validation.ts:11:22:11:33 | validatedObj |
+| parameter | global/validation.ts:11:56:11:66 | unvalidated |
+| parameter | global/validation.ts:17:22:17:22 | x |
 | parameter | local/customDecorator.ts:6:12:6:41 | request ... ryParam |
 | parameter | local/customPipe.ts:5:15:5:19 | value |
 | parameter | local/customPipe.ts:13:15:13:19 | value |
@@ -58,8 +61,8 @@ requestInputAccess
 | parameter | local/validation.ts:42:22:42:33 | validatedObj |
 | parameter | local/validation.ts:42:56:42:66 | unvalidated |
 responseSendArgument
-| global/validation.ts:7:31:7:41 | unvalidated |
-| global/validation.ts:8:12:8:27 | validatedObj.key |
+| global/validation.ts:12:31:12:41 | unvalidated |
+| global/validation.ts:13:12:13:27 | validatedObj.key |
 | local/customDecorator.ts:19:12:19:16 | value |
 | local/customDecorator.ts:24:12:24:16 | value |
 | local/customPipe.ts:21:16:21:29 | '' + sanitized |

javascript/ql/test/library-tests/frameworks/Nest/test.ql

@@ -1,5 +1,6 @@
 import javascript
 private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
+private import utils.test.InlineFlowTest
 
 query Http::RouteHandler routeHandler() { any() }
 
@@ -17,3 +18,13 @@ query RemoteFlowSource requestInputAccess(string kind) {
 query Http::ResponseSendArgument responseSendArgument() { any() }
 
 query ServerSideUrlRedirect::Sink redirectSink() { any() }
+
+module TestConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node node) { node instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node node) {
+    exists(DataFlow::CallNode call | call.getCalleeName() = "sink" and node = call.getArgument(0))
+  }
+}
+
+import ValueFlowTest<TestConfig>