diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll index d8ab27b16..2e96cc1be 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll @@ -177,7 +177,27 @@ class ServiceInstanceFromConstructor extends ServiceInstance { } /** - * A read to `this` variable which represents the service whose definition encloses this variable access. + * A read to `this` variable which represents the service whose definition encloses + * this variable access. + * e.g.1. Given this code: + * ``` javascript + * const cds = require("@sap/cds"); + * module.exports = class SomeService extends cds.ApplicationService { + * init() { + * this.on("SomeEvent", (req) => { ... } ) + * } + * } + * ``` + * This class captures the access to the `this` variable as in `this.on(...)`. + * + * e.g.2. Given this code: + * ``` javascript + * const cds = require('@sap/cds'); + * module.exports = cds.service.impl (function() { + * this.on("SomeEvent", (req) => { ... }) + * }) + * ``` + * This class captures the access to the `this` variable as in `this.on(...)`. */ class ServiceInstanceFromThisNode extends ServiceInstance, ThisNode { UserDefinedApplicationService userDefinedApplicationService; @@ -294,12 +314,56 @@ class DbServiceInstanceFromCdsConnectTo extends ServiceInstanceFromCdsConnectTo, override UserDefinedApplicationService getDefinition() { none() } } +/** + * The 0-th parameter of an exported closure that represents the service being implemented. e.g. + * ``` javascript + * const cds = require('@sap/cds') + * module.exports = (srv) => { + * srv.on("SomeEvent1", (req) => { ... }) + * } + * ``` + * This class captures the `srv` parameter of the exported arrow function. Also see + * `ServiceInstanceFromImplMethodCallClosureParameter` which is similar to this. + */ +class ServiceInstanceFromExportedClosureParameter extends ServiceInstance, ParameterNode { + ExportedClosureApplicationServiceDefinition exportedClosure; + + ServiceInstanceFromExportedClosureParameter() { this = exportedClosure.getParameter(0) } + + override UserDefinedApplicationService getDefinition() { result = exportedClosure } +} + +/** + * The 0-th parameter of a callback (usually an arrow function) passed to `cds.service.impl` that + * represents the service being implemented. e.g. + * ``` javascript + * const cds = require('@sap/cds') + * module.exports = cds.service.impl((srv) => { + * srv.on("SomeEvent1", (req) => { ... }) + * }) + * ``` + * This class captures the `srv` parameter of the exported arrow function. Also see + * `ServiceInstanceFromExportedClosureParameter` which is similar to this. + */ +class ServiceInstanceFromImplMethodCallClosureParameter extends ServiceInstance, ParameterNode { + ImplMethodCallApplicationServiceDefinition implMethodCallApplicationServiceDefinition; + + ServiceInstanceFromImplMethodCallClosureParameter() { + this = implMethodCallApplicationServiceDefinition.getInitFunction().getParameter(0) + } + + override UserDefinedApplicationService getDefinition() { + result = implMethodCallApplicationServiceDefinition + } +} + /** * A call to `before`, `on`, or `after` on an `cds.ApplicationService`. * It registers an handler to be executed when an event is fired, * to do something with the incoming request or event as its parameter. */ class HandlerRegistration extends MethodCallNode { + /** The instance of the service a handler is registered on. */ ServiceInstance srv; string methodName; @@ -308,6 +372,9 @@ class HandlerRegistration extends MethodCallNode { methodName = ["before", "on", "after"] } + /** + * Gets the instance of the service a handler is registered on. + */ ServiceInstance getService() { result = srv } /** @@ -347,7 +414,7 @@ class HandlerRegistration extends MethodCallNode { /** * The first parameter of a handler, representing the request object received either directly - * from a user, or from another service that may be internal (defined in the same application) + * from a user, or from another service that may be internal (defined in the same application) * or external (defined in another application, or even served from a different server). * e.g. * ``` javascript @@ -356,7 +423,7 @@ class HandlerRegistration extends MethodCallNode { * this.before("SomeEvent", "SomeEntity", (req, next) => { ... }); * this.after("SomeEvent", "SomeEntity", (req, next) => { ... }); * } - * ``` + * ``` * All parameters named `req` above are captured. Also see `HandlerParameterOfExposedService` * for a subset of this class that is only about handlers exposed to some protocol. */ @@ -506,7 +573,7 @@ abstract class UserDefinedApplicationService extends UserDefinedService { /** * Holds if this service supports access from the outside through any kind of protocol. */ - predicate isExposed() { not this.isInternal() } + predicate isExposed() { exists(this.getCdsDeclaration()) and not this.isInternal() } /** * Holds if this service does not support access from the outside through any kind of protocol, thus being internal only. @@ -539,9 +606,21 @@ class ES6ApplicationServiceDefinition extends ClassNode, UserDefinedApplicationS /** * Subclassing `cds.ApplicationService` via a call to `cds.service.impl`. - * ```js + * e.g.1. Given this code: + * ``` javascript + * const cds = require('@sap/cds') + * module.exports = cds.service.impl (function() { + * this.on("SomeEvent1", (req) => { ... }) + * }) + * ``` + * This class captures the call `cds.service.impl (function() { ... })`. + * + * e.g.2. Given this code: + * ``` javascript * const cds = require('@sap/cds') - * module.exports = cds.service.impl (function() { ... }) + * module.exports = cds.service.impl ((srv) => { + * srv.on("SomeEvent1", (req) => { ... }) + * }) * ``` */ class ImplMethodCallApplicationServiceDefinition extends MethodCallNode, @@ -554,6 +633,40 @@ class ImplMethodCallApplicationServiceDefinition extends MethodCallNode, override FunctionNode getInitFunction() { result = this.getArgument(0) } } +/** + * A user-defined application service that comes in a form of an exported + * closure. e.g. Given the below code, + * ``` javascript + * const cds = require("@sap/cds"); + * + * module.exports = (srv) => { + * srv.before("SomeEvent1", "SomeEntity", (req, res) => { ... }) + * srv.on("SomeEvent2", (req) => { ... } ) + * srv.after("SomeEvent3", (req) => { ... } ) + * } + * ``` + * This class captures the entire `(srv) => { ... }` function that is + * exported. + */ +class ExportedClosureApplicationServiceDefinition extends FunctionNode, + UserDefinedApplicationService +{ + ExportedClosureApplicationServiceDefinition() { + /* + * ==================== HACK ==================== + * See issue #221. + */ + + exists(PropWrite moduleExports | + moduleExports.getBase().asExpr().(VarAccess).getName() = "module" and + moduleExports.getPropertyName() = "exports" and + this = moduleExports.getRhs() + ) + } + + override FunctionNode getInitFunction() { result = this } +} + abstract class InterServiceCommunicationMethodCall extends MethodCallNode { string name; ServiceInstance recipient; diff --git a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll index d4fee4a5d..66e4001d3 100644 --- a/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll +++ b/javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll @@ -30,7 +30,9 @@ class HandlerParameterOfExposedService extends HandlerParameter { * is known. */ - not exists(this.getHandler().getHandlerRegistration().getService().getDefinition()) + not exists( + this.getHandler().getHandlerRegistration().getService().getDefinition().getCdsDeclaration() + ) } } @@ -54,7 +56,9 @@ class UserProvidedPropertyReadOfHandlerParameterOfExposedService extends RemoteF UserProvidedPropertyReadOfHandlerParameterOfExposedService() { /* 1. `req.(data|params|headers|id)` */ - this = handlerParameterOfExposedService.getAPropertyRead(["data", "params", "headers", "id"]) + this = + handlerParameterOfExposedService + .getAPropertyRead(["data", "params", "headers", "id", "_queryOptions"]) or /* 2. APIs stemming from `req.http.req`: Defined by Express.js */ exists(PropRead reqHttpReq | diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.expected b/javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.expected similarity index 91% rename from javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.expected rename to javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.expected index 4878319a2..5905d5d55 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.expected +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.expected @@ -9,6 +9,7 @@ | srv/service1.js:52:29:52:31 | req | | srv/service1.js:58:29:58:31 | req | | srv/service1.js:64:29:64:31 | req | +| srv/service1.js:70:30:70:32 | req | | srv/service2.js:4:27:4:29 | msg | | srv/service3.js:5:29:5:31 | req | | srv/service3.js:11:29:11:31 | req | @@ -19,3 +20,4 @@ | srv/service3.js:51:29:51:31 | req | | srv/service3.js:57:29:57:31 | req | | srv/service3.js:63:29:63:31 | req | +| srv/service3.js:69:30:69:32 | req | diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.ql b/javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.ql similarity index 100% rename from javascript/frameworks/cap/test/models/cds/remoteflowsources/ExposedServices.ql rename to javascript/frameworks/cap/test/models/cds/remoteflowsources/HandlerParameterOfExposedService.ql diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected index a3c84483b..06255b897 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected @@ -13,6 +13,7 @@ | srv/service1.js:34:31:34:61 | req.htt ... eProp") | | srv/service1.js:35:31:35:60 | req.htt ... eProp") | | srv/service1.js:41:29:41:34 | req.id | +| srv/service1.js:47:29:47:45 | req._queryOptions | | srv/service2.js:5:31:5:38 | msg.data | | srv/service3.js:6:33:6:40 | req.data | | srv/service3.js:12:33:12:42 | req.params | @@ -29,3 +30,4 @@ | srv/service3.js:33:31:33:61 | req.htt ... eProp") | | srv/service3.js:34:31:34:60 | req.htt ... eProp") | | srv/service3.js:40:29:40:34 | req.id | +| srv/service3.js:46:29:46:45 | req._queryOptions | diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service1.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service1.js index 05ad0d761..caed65129 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service1.js +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service1.js @@ -44,24 +44,30 @@ module.exports = class Service1 extends cds.ApplicationService { }); this.on("send6", async (req) => { - const messageToPass = req.locale; // SAFE: Not a taint source, Exposed service + const messageToPass = req._queryOptions; // UNSAFE: Taint source, Exposed service const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send7", async (req) => { - const messageToPass = req.tenant; // SAFE: Not a taint source, Exposed service + const messageToPass = req.locale; // SAFE: Not a taint source, Exposed service const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send8", async (req) => { - const messageToPass = req.timestamp; // SAFE: Not a taint source, Exposed service + const messageToPass = req.tenant; // SAFE: Not a taint source, Exposed service const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send9", async (req) => { + const messageToPass = req.timestamp; // SAFE: Not a taint source, Exposed service + const Service2 = await cds.connect.to("service-2"); + Service2.send("send2", { messageToPass }); + }); + + this.on("send10", async (req) => { const messageToPass = req.user; // SAFE: Not a taint source, Exposed service const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3.js index 45a4afe12..060561482 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3.js +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3.js @@ -43,24 +43,30 @@ class Service3 extends cds.ApplicationService { }); this.on("send6", async (req) => { - const messageToPass = req.locale; // SAFE: Not a taint source, Exposed service (fallback) + const messageToPass = req._queryOptions; // UNSAFE: Taint source, Exposed service const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send7", async (req) => { - const messageToPass = req.tenant; // SAFE: Not a taint source, Exposed service (fallback) + const messageToPass = req.locale; // SAFE: Not a taint source, Exposed service (fallback) const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send8", async (req) => { - const messageToPass = req.timestamp; // SAFE: Not a taint source, Exposed service (fallback) + const messageToPass = req.tenant; // SAFE: Not a taint source, Exposed service (fallback) const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); this.on("send9", async (req) => { + const messageToPass = req.timestamp; // SAFE: Not a taint source, Exposed service (fallback) + const Service2 = await cds.connect.to("service-2"); + Service2.send("send2", { messageToPass }); + }); + + this.on("send10", async (req) => { const messageToPass = req.user; // SAFE: Not a taint source, Exposed service (fallback) const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.js index 76dd7d6cd..343d57aa9 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.js +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.js @@ -41,5 +41,11 @@ module.exports = class Service4 extends cds.ApplicationService { const Service2 = await cds.connect.to("service-2"); Service2.send("send2", { messageToPass }); }); + + this.on("send6", async (req) => { + const messageToPass = req._queryOptions; // UNSAFE: Taint source, Exposed service + const Service2 = await cds.connect.to("service-2"); + Service2.send("send2", { messageToPass }); + }); } }; diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/service1.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service1.js new file mode 100644 index 000000000..a90a1f819 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service1.js @@ -0,0 +1,9 @@ +const cds = require("@sap/cds"); + +class Service1 extends cds.ApplicationService { // ES6ApplicationServiceDefinition + init() { + return super.init() + } +} + +module.exports = Service1 \ No newline at end of file diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/service2.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service2.js new file mode 100644 index 000000000..441c11207 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service2.js @@ -0,0 +1,7 @@ +const cds = require("@sap/cds"); + +module.exports = class LogService extends cds.Service { // UserDefinedService + init() { + return super.init() + } +} \ No newline at end of file diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/service3.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service3.js new file mode 100644 index 000000000..73f7fb901 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service3.js @@ -0,0 +1,7 @@ +const cds = require("@sap/cds"); + +module.exports = cds.service.impl(function () { // ImplMethodCallApplicationServiceDefinition + this.on("SomeEvent1", (req) => { + /* ... */ + }); +}); diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/service4.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service4.js new file mode 100644 index 000000000..4fe1ec76b --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service4.js @@ -0,0 +1,8 @@ +const cds = require("@sap/cds"); + +module.exports = cds.service.impl((srv) => { // ImplMethodCallApplicationServiceDefinition + this.on("SomeEvent1", (req) => { + /* ... */ + }); +}); + diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/service5.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service5.js new file mode 100644 index 000000000..3bbf97340 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/service5.js @@ -0,0 +1,7 @@ +const cds = require("@sap/cds"); + +module.exports = (srv) => { // ExportedClosureApplicationServiceDefinition + srv.on("SomeEvent1", (req) => { + /* ... */ + }); +}; diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.expected b/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.expected index ffa9b81e3..743429c93 100644 --- a/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.expected +++ b/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.expected @@ -1,2 +1,5 @@ -| userdefinedservice.js:3:1:7:1 | class B ... )\\n }\\n} | -| userdefinedservice.js:10:18:14:1 | class L ... )\\n }\\n} | +| service1.js:3:1:7:1 | class S ... )\\n }\\n} | +| service2.js:3:18:7:1 | class L ... )\\n }\\n} | +| service3.js:3:18:7:2 | cds.ser ... });\\n}) | +| service4.js:3:18:7:2 | cds.ser ... });\\n}) | +| service5.js:3:18:7:1 | (srv) = ... });\\n} | diff --git a/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.js b/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.js deleted file mode 100644 index 68efdc809..000000000 --- a/javascript/frameworks/cap/test/models/cds/userdefinedservice/userdefinedservice.js +++ /dev/null @@ -1,14 +0,0 @@ -const cds = require("@sap/cds"); - -class BooksService extends cds.ApplicationService { - init() { - return super.init() - } -} -module.exports = BooksService - -module.exports = class LogService extends cds.Service { - init() { - return super.init() - } -}