JS: Replace another use of Type

Author: asgerf

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

@@ -5,6 +5,7 @@
 import javascript
 private import semmle.javascript.security.dataflow.ServerSideUrlRedirectCustomizations
 private import semmle.javascript.dataflow.internal.PreCallGraphStep
+private import semmle.javascript.internal.NameResolution
 private import semmle.javascript.internal.TypeResolution
 
 /**
@@ -317,14 +318,6 @@ module NestJS {
     }
   }
 
-  private predicate isStringType(Type type) {
-    type instanceof StringType
-    or
-    type instanceof AnyType
-    or
-    isStringType(type.(PromiseType).getElementType().unfold())
-  }
-
   /**
    * A return value from a route handler, seen as an argument to `res.send()`.
    *
@@ -343,10 +336,10 @@ module NestJS {
     ReturnValueAsResponseSend() {
       handler.isReturnValueReflected() and
       this = handler.getAReturn() and
-      // Only returned strings are sinks
-      not exists(Type type |
-        type = this.asExpr().getType() and
-        not isStringType(type.unfold())
+      // Only returned strings are sinks. If we can find a type for the return value, it must be string-like.
+      not exists(NameResolution::Node type |
+        TypeResolution::valueHasType(this.asExpr(), type) and
+        not TypeResolution::hasUnderlyingStringOrAnyType(type)
       )
     }
 

javascript/ql/lib/semmle/javascript/internal/TypeResolution.qll

@@ -360,4 +360,25 @@ module TypeResolution {
    * Holds if the given value has a type that implied it is a Promise object. Does not hold for unions unless all parts of the union are promises.
    */
   predicate valueHasPromiseType = ValueHasProperty<isPromiseType/1>::valueHasProperty/1;
+
+  /**
+   * Holds if `type` contains `string` or `any`, possibly wrapped in a promise.
+   */
+  predicate hasUnderlyingStringOrAnyType(Node type) {
+    type.(TypeAnnotation).isStringy()
+    or
+    type.(TypeAnnotation).isAny()
+    or
+    type instanceof StringLiteralTypeExpr
+    or
+    type instanceof TemplateLiteralTypeExpr
+    or
+    exists(Node mid | hasUnderlyingStringOrAnyType(mid) |
+      TypeFlow::step(mid, type)
+      or
+      UnderlyingTypes::underlyingTypeStep(mid, type)
+      or
+      type = unwrapPromiseType(mid)
+    )
+  }
 }

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

@@ -71,6 +71,9 @@ responseSendArgument
 | local/customPipe.ts:37:16:37:31 | '' + unsanitized |
 | local/customPipe.ts:42:16:42:31 | '' + unsanitized |
 | local/customPipe.ts:48:16:48:31 | '' + unsanitized |
+| local/routes.ts:7:12:7:16 | 'foo' |
+| local/routes.ts:12:12:12:16 | 'foo' |
+| local/routes.ts:17:12:17:16 | 'foo' |
 | local/routes.ts:32:31:32:31 | x |
 | local/routes.ts:33:31:33:38 | queryObj |
 | local/routes.ts:34:31:34:34 | name |