diff --git a/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll b/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll index d6bcb9ddd400..fcd8da5c8931 100644 --- a/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll +++ b/javascript/ql/lib/semmle/javascript/frameworks/Nest.qll @@ -447,6 +447,61 @@ module NestJS { } } + /** + * A NestJS Middleware Class + */ + private class NestMiddlewareClass extends DataFlow::ClassNode { + NestMiddlewareClass() { + exists(ClassDefinition cls | + this = cls.flow() and + cls.getASuperInterface().hasUnderlyingType("@nestjs/common", "NestMiddleware") + ) + } + + DataFlow::FunctionNode getUseFunction() { result = this.getInstanceMethod("use") } + } + + /** + * A NestJS Middleware Class route handler (the `use` method) + */ + private class MiddlewareRouteHandler extends Http::RouteHandler, DataFlow::FunctionNode { + MiddlewareRouteHandler() { this = any(NestMiddlewareClass m).getUseFunction() } + + override Http::HeaderDefinition getAResponseHeader(string name) { none() } + + /** + * Gets the request object used by this route + */ + DataFlow::ParameterNode getRequest() { result = this.getParameter(0) } + + /** + * Gets the response object used by this route + */ + DataFlow::ParameterNode getResponse() { result = this.getParameter(1) } + } + + /** + * A source of `express` request objects for NestJS middlewares + */ + private class MiddlewareRequestSource extends Express::RequestSource { + MiddlewareRouteHandler middlewareRouteHandler; + + MiddlewareRequestSource() { this = middlewareRouteHandler.getRequest() } + + override Http::RouteHandler getRouteHandler() { result = middlewareRouteHandler } + } + + /** + * A source of `express` response objects for NestJS middlewares + */ + private class MiddlewareResponseSource extends Express::ResponseSource { + MiddlewareRouteHandler middlewareRouteHandler; + + MiddlewareResponseSource() { this = middlewareRouteHandler.getResponse() } + + override Http::RouteHandler getRouteHandler() { result = middlewareRouteHandler } + } + /** * A value passed in the `providers` array in: * ```js @@ -455,21 +510,53 @@ module NestJS { * ``` */ private DataFlow::Node providerTuple() { - result = - DataFlow::moduleImport("@nestjs/common") - .getAPropertyRead("Module") - .getACall() - .getOptionArgument(0, "providers") - .getALocalSource() - .(DataFlow::ArrayCreationNode) - .getAnElement() + exists(DataFlow::CallNode moduleCall | + moduleCall = DataFlow::moduleImport("@nestjs/common").getAPropertyRead("Module").getACall() and + result = providerTupleAux(moduleCall.getArgument(0).getALocalSource()) + ) + } + + private DataFlow::Node providerTupleAux(DataFlow::ObjectLiteralNode o) { + ( + result = + o.getAPropertyWrite("providers") + .getRhs() + .getALocalSource() + .(DataFlow::ArrayCreationNode) + .getAnElement() + or + result = + providerTupleAux(o.getAPropertyWrite("imports") + .getRhs() + .getALocalSource() + .(DataFlow::ArrayCreationNode) + .getAnElement() + .(DataFlow::CallNode) + .getCalleeNode() + .getAFunctionValue() + .getFunction() + .getAReturnedExpr() + .flow()) + ) + } + + private DataFlow::Node getConcreteClassFromProviderTuple(DataFlow::SourceNode tuple) { + result = tuple.getAPropertyWrite("useClass").getRhs() + or + exists(DataFlow::FunctionNode f | + f = tuple.getAPropertyWrite("useFactory").getRhs().getAFunctionValue() and + result.getAstNode() = f.getFunction().getAReturnedExpr().getType().(ClassType).getClass() + ) + or + result.getAstNode() = + tuple.getAPropertyWrite("useValue").getRhs().asExpr().getType().(ClassType).getClass() } 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() + concreteClass = getConcreteClassFromProviderTuple(tuple) ) } diff --git a/javascript/ql/test/library-tests/frameworks/Nest/global/app.module.ts b/javascript/ql/test/library-tests/frameworks/Nest/global/app.module.ts index 2c230821a632..b8793dc57895 100644 --- a/javascript/ql/test/library-tests/frameworks/Nest/global/app.module.ts +++ b/javascript/ql/test/library-tests/frameworks/Nest/global/app.module.ts @@ -1,12 +1,27 @@ import { Module } from '@nestjs/common'; import { Controller } from './validation'; -import { Foo } from './foo.interface'; -import { FooImpl } from './foo.impl'; +import { Imports } from './imports'; +import { Foo, Foo2, Foo3 } from './foo.interface'; +import { FooImpl, Foo2Impl, Foo3Impl } from './foo.impl'; + +const foo3 = new Foo3Impl() @Module({ - controllers: [Controller], - providers: [{ - provide: Foo, useClass: FooImpl - }], + controllers: [Controller], + imports: [Imports.forRoot()], + providers: [ + { + provide: Foo, + useClass: FooImpl + }, + { + provide: Foo2, + useFactory: () => new Foo2Impl() + }, + { + provide: Foo3, + useValue: foo3 + } + ], }) export class AppModule { } diff --git a/javascript/ql/test/library-tests/frameworks/Nest/global/foo.impl.ts b/javascript/ql/test/library-tests/frameworks/Nest/global/foo.impl.ts index 979389a3804c..9e54bc4774e9 100644 --- a/javascript/ql/test/library-tests/frameworks/Nest/global/foo.impl.ts +++ b/javascript/ql/test/library-tests/frameworks/Nest/global/foo.impl.ts @@ -1,7 +1,25 @@ -import { Foo } from "./foo.interface"; +import { Foo, Foo2, Foo3, Foo4 } from "./foo.interface"; export class FooImpl extends Foo { fooMethod(x: string) { sink(x); // $ hasValueFlow=x } } + +export class Foo2Impl extends Foo2 { + fooMethod(x: string) { + sink(x); // $ hasValueFlow=x + } +} + +export class Foo3Impl extends Foo3 { + fooMethod(x: string) { + sink(x); // $ hasValueFlow=x + } +} + +export class Foo4Impl extends Foo4 { + fooMethod(x: string) { + sink(x); // $ hasValueFlow=x + } +} \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/Nest/global/foo.interface.ts b/javascript/ql/test/library-tests/frameworks/Nest/global/foo.interface.ts index f22529c2d185..b3d18f2749a3 100644 --- a/javascript/ql/test/library-tests/frameworks/Nest/global/foo.interface.ts +++ b/javascript/ql/test/library-tests/frameworks/Nest/global/foo.interface.ts @@ -1,3 +1,15 @@ export abstract class Foo { abstract fooMethod(x: string): void; } + +export abstract class Foo2 { + abstract fooMethod(x: string): void; +} + +export abstract class Foo3 { + abstract fooMethod(x: string): void; +} + +export abstract class Foo4 { + abstract fooMethod(x: string): void; +} diff --git a/javascript/ql/test/library-tests/frameworks/Nest/global/imports.ts b/javascript/ql/test/library-tests/frameworks/Nest/global/imports.ts new file mode 100644 index 000000000000..1df36111161c --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/Nest/global/imports.ts @@ -0,0 +1,16 @@ +import { DynamicModule } from '@nestjs/common'; +import { Foo4Impl } from './foo.impl'; +import { Foo4 } from './foo.interface'; + +export class Imports { + static forRoot(): DynamicModule { + return { + providers: [ + { + provide: Foo4, + useClass: Foo4Impl, + }, + ], + }; + } +} diff --git a/javascript/ql/test/library-tests/frameworks/Nest/global/validation.ts b/javascript/ql/test/library-tests/frameworks/Nest/global/validation.ts index 1872b5a51b7d..f6046e4651a1 100644 --- a/javascript/ql/test/library-tests/frameworks/Nest/global/validation.ts +++ b/javascript/ql/test/library-tests/frameworks/Nest/global/validation.ts @@ -1,10 +1,10 @@ import { Get, Query } from '@nestjs/common'; import { IsIn } from 'class-validator'; -import { Foo } from './foo.interface'; +import { Foo, Foo2, Foo3, Foo4 } from './foo.interface'; export class Controller { constructor( - private readonly foo: Foo + private readonly foo: Foo, private readonly foo2: Foo2, private readonly foo3: Foo3, private readonly foo4: Foo4 ) { } @Get() @@ -16,6 +16,9 @@ export class Controller { @Get() route2(@Query('x') x: string) { this.foo.fooMethod(x); + this.foo2.fooMethod(x); + this.foo3.fooMethod(x); + this.foo4.fooMethod(x); } } diff --git a/javascript/ql/test/library-tests/frameworks/Nest/local/middleware.ts b/javascript/ql/test/library-tests/frameworks/Nest/local/middleware.ts new file mode 100644 index 000000000000..f7f7104c68cc --- /dev/null +++ b/javascript/ql/test/library-tests/frameworks/Nest/local/middleware.ts @@ -0,0 +1,12 @@ +import { Injectable, NestMiddleware } from '@nestjs/common'; +import { Response, NextFunction } from 'express'; +import { CustomRequest } from '@randomPackage/request'; + +@Injectable() +export class LoggerMiddleware implements NestMiddleware { + // The request can be a custom type that extends the express Request + use(req: CustomRequest, res: Response, next: NextFunction) { + console.log(req.query.abc); + next(); + } +} \ No newline at end of file diff --git a/javascript/ql/test/library-tests/frameworks/Nest/test.expected b/javascript/ql/test/library-tests/frameworks/Nest/test.expected index db49fc95eba9..ea74b306b366 100644 --- a/javascript/ql/test/library-tests/frameworks/Nest/test.expected +++ b/javascript/ql/test/library-tests/frameworks/Nest/test.expected @@ -1,7 +1,7 @@ testFailures routeHandler | global/validation.ts:11:3:14:3 | route1( ... OK\\n } | -| global/validation.ts:17:3:19:3 | route2( ... x);\\n } | +| global/validation.ts:17:3:22: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 } | @@ -10,6 +10,7 @@ routeHandler | local/customPipe.ts:36:5:38:5 | propaga ... K\\n } | | local/customPipe.ts:41:5:43:5 | propaga ... K\\n } | | local/customPipe.ts:47:5:49:5 | propaga ... K\\n } | +| local/middleware.ts:8:3:11:3 | use(req ... ();\\n } | | local/routes.ts:6:3:8:3 | getFoo( ... o';\\n } | | local/routes.ts:11:3:13:3 | postFoo ... o';\\n } | | local/routes.ts:16:3:18:3 | getRoot ... o';\\n } | @@ -29,9 +30,11 @@ routeHandler | local/validation.ts:42:3:45:3 | route6( ... OK\\n } | requestSource | local/customDecorator.ts:5:21:5:51 | ctx.swi ... quest() | +| local/middleware.ts:8:7:8:9 | req | | local/routes.ts:30:12:30:14 | req | | local/routes.ts:61:23:61:25 | req | responseSource +| local/middleware.ts:8:27:8:29 | res | | local/routes.ts:61:35:61:37 | res | | local/routes.ts:62:5:62:25 | res.sen ... uery.x) | requestInputAccess @@ -44,6 +47,7 @@ requestInputAccess | 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 | +| parameter | local/middleware.ts:9:17:9:29 | req.query.abc | | parameter | local/routes.ts:27:17:27:17 | x | | parameter | local/routes.ts:28:14:28:21 | queryObj | | parameter | local/routes.ts:29:20:29:23 | name |