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 e6745b1fb..d704a6910 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 @@ -217,27 +217,28 @@ class CdsServeWithCall extends MethodCallNode { */ class ServiceInstanceFromServeWithParameter extends ServiceInstance { CdsServeCall cdsServe; + CdsServeWithCall cdsServeWith; ServiceInstanceFromServeWithParameter() { - exists(CdsServeWithCall cdsServeWith | - /* - * cds.serve('./srv/some-service1').with ((srv) => { // Parameter `srv` is captured. - * srv.on ('READ','SomeEntity1', (req) => req.reply([...])) - * }) - */ + /* + * cds.serve('./srv/some-service1').with ((srv) => { // Parameter `srv` is captured. + * srv.on ('READ','SomeEntity1', (req) => req.reply([...])) + * }) + */ - this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr) - or - /* - * cds.serve('./srv/some-service2').with (function() { // Parameter `this` is captured. - * this.on ('READ','SomeEntity2', (req) => req.reply([...])) - * }) - */ + this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr) + or + /* + * cds.serve('./srv/some-service2').with (function() { // Parameter `this` is captured. + * this.on ('READ','SomeEntity2', (req) => req.reply([...])) + * }) + */ - this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow() - ) + this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow() } + HandlerRegistration getAHandlerRegistration() { result = cdsServeWith.getAHandlerRegistration() } + override UserDefinedApplicationService getDefinition() { /* 1. The argument to cds.serve is "all" */ cdsServe.getServiceRepresentation().getStringValue() = "all" and @@ -344,6 +345,19 @@ class HandlerRegistration extends MethodCallNode { predicate isAfter() { methodName = "after" } } +/** + * A parameter of a handler + */ +class HandlerParameter instanceof ParameterNode { + Handler handler; + + HandlerParameter() { this = handler.getParameter(0) } + + Handler getHandler() { result = handler } + + string toString() { result = super.toString() } +} + /** * The handler that implements a service's logic to deal with the incoming request or message when a certain event is fired. * It is the last argument to the method calls that registers the handler: either `srv.before`, `srv.on`, or `srv.after`. @@ -351,16 +365,10 @@ class HandlerRegistration extends MethodCallNode { * or is of type `cds.Request` and handles the event synchronously. */ class Handler extends FunctionNode { - UserDefinedApplicationService srv; HandlerRegistration handlerRegistration; Handler() { this = handlerRegistration.getAnArgument() } - /** - * Gets the service registering this handler. - */ - UserDefinedApplicationService getDefiningService() { result = srv } - /** * Gets a name of one of the event this handler is registered for. */ @@ -375,6 +383,8 @@ class Handler extends FunctionNode { * Gets the request that this handler is registered for, as represented as its first parameter. */ CdsRequest getRequest() { result = this.getParameter(0) } + + HandlerRegistration getHandlerRegistration() { result = handlerRegistration } } class CdsRequest extends ParameterNode { @@ -822,7 +832,7 @@ class HandlerParameterData instanceof PropRead { string dataName; HandlerParameterData() { - this = handlerParameter.getAPropertyRead("data").getAPropertyRead(dataName) + this = handlerParameter.(SourceNode).getAPropertyRead("data").getAPropertyRead(dataName) } /** @@ -841,7 +851,7 @@ class HandlerParameterData instanceof PropRead { CdlActionOrFunction cdlActionOrFunction, HandlerRegistration handlerRegistration | handlerRegistration = userDefinedApplicationService.getAHandlerRegistration() and - handlerRegistration = handlerParameter.getHandlerRegistration() and + handlerRegistration = handlerParameter.getHandler().getHandlerRegistration() and handlerRegistration.getAnEventName() = cdlActionOrFunction.getUnqualifiedName() and cdlActionOrFunction = userDefinedApplicationService.getCdsDeclaration().getAnActionOrFunction() and 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 208b25bd6..7f41a850c 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 @@ -2,65 +2,35 @@ import javascript import advanced_security.javascript.frameworks.cap.CDS /** - * A parameter of a handler registered for a service on an event. e.g. + * Either of: + * a parameter of a handler registered for an (exposed) service on an event. e.g. * ```javascript * this.on("SomeEvent", "SomeEntity", (req) => { ... }); - * this.before("SomeEvent", "SomeEntity", (req, next) => { ... }); // only `req` is captured + * this.before("SomeEvent", "SomeEntity", (req, next) => { ... }); * SomeService.on("SomeEvent", "SomeEntity", (msg) => { ... }); * SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... }); * ``` - * All the parameters named `req` and `msg` are captured in the above example. - */ -class HandlerParameter extends ParameterNode, RemoteFlowSource { - Handler handler; - HandlerRegistration handlerRegistration; - - HandlerParameter() { - exists(UserDefinedApplicationService service | - handler = handlerRegistration.getHandler() and - this = handler.getParameter(0) and - service.getAHandlerRegistration() = handlerRegistration and - service.isExposed() - ) - } - - override string getSourceType() { - result = "Parameter of an event handler belonging to an exposed service" - } - - /** - * Gets the handler this is a parameter of. - */ - Handler getHandler() { result = handler } - - /** - * Gets the handler registration registering the handler it is a parameter of. - */ - HandlerRegistration getHandlerRegistration() { result = handlerRegistration } -} - -/** - * A service may be described only in a CDS file, but event handlers may still be registered in a format such as: + * OR + * a handler parameter that is not connected to a service + * possibly due to cds compilation failure + * or non explicit service references in source. e.g. * ```javascript - * module.exports = srv => { - * srv.before('CREATE', 'Media', req => { // an entity name is used to describe which to register this handler to. - * ... - * }); - * } + * cds.serve('./test-service').with((srv) => { + * srv.after('READ', req => req.target.data) //req + * }) * ``` - * parameters named `req` are captured in the above example. */ -class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource { - ServiceinCDSHandlerParameter() { - exists(MethodCallNode m, CdlEntity entity, string entityName | - entity.getName().regexpReplaceAll(".*\\.", "") = entityName and - m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName and - this = m.getArgument(m.getNumArgument() - 1).(FunctionNode).getParameter(0) and - m.getMethodName() in ["on", "before", "after"] - ) +class HandlerParameterOfExposedService extends RemoteFlowSource, HandlerParameter { + HandlerParameterOfExposedService() { + this.getHandler().getHandlerRegistration().getService().getDefinition().isExposed() + or + /* no precise service definition is known */ + not exists(this.getHandler().getHandlerRegistration().getService().getDefinition()) } + override string toString() { result = HandlerParameter.super.toString() } + override string getSourceType() { - result = "Parameter of an event handler belonging to an exposed service defined in a cds file" + result = "Parameter of an event handler belonging to an exposed service" } } diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/db/schema.cds b/javascript/frameworks/cap/test/models/cds/remoteflowsources/db/schema.cds index 1d4202fb1..cce933984 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/db/schema.cds +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/db/schema.cds @@ -8,4 +8,8 @@ entity Entity1 { entity Entity2 { Attribute3 : String(100); Attribute4 : String(100) +} + +entity Entity4 { + Attribute4 : String(100) } \ No newline at end of file diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected index 0b94e0bba..dcc8f9d60 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected @@ -1,2 +1,12 @@ | srv/service1.js:6:29:6:31 | req | | srv/service2.js:5:27:5:29 | msg | +| srv/service3nocds.js:6:43:6:45 | req | +| srv/service3nocds.js:7:34:7:36 | req | +| srv/service3nocds.js:11:28:11:30 | req | +| srv/service3nocds.js:12:26:12:28 | req | +| srv/service3nocds.js:19:34:19:36 | req | +| srv/service4withcds.js:5:38:5:40 | req | +| srv/service4withcds.js:6:43:6:45 | req | +| srv/service4withcds.js:14:33:14:35 | req | +| srv/service4withcds.js:15:38:15:40 | req | +| srv/service4withcds.js:16:23:16:25 | req | diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.ql b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.ql index 9381dbed0..4643a0c0c 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.ql +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.ql @@ -2,4 +2,4 @@ import javascript import advanced_security.javascript.frameworks.cap.RemoteFlowSources from RemoteFlowSource source -select source \ No newline at end of file +select source diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js new file mode 100644 index 000000000..e44cb18d3 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js @@ -0,0 +1,20 @@ +//this service unit test is a replica of requesthandler.js +const cds = require("@sap/cds"); +class BooksService extends cds.ApplicationService { + init() { + const { Books, Authors } = this.entities + this.on('READ', [Books, Authors], req => req.target.data) //req + this.on('UPDATE', Books, req => { //req + let [ID] = req.params + return Object.assign(Books.data[ID], req.data) + }) + this.after('READ', req => req.target.data) //req + this.before('*', req => req.target.data) //req + return super.init() + } +} +module.exports = BooksService + +cds.serve('./test-service').with((srv) => { + srv.before('READ', 'Books', (req) => req.reply([])) //req +}) \ No newline at end of file diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.cds b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.cds new file mode 100644 index 000000000..e46375879 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.cds @@ -0,0 +1,11 @@ +using { advanced_security.log_injection.sample_entities as db_schema } from '../db/schema'; + +service Service4 @(path: '/service-4') { + /* Entity to send READ/GET about. */ + entity Service4Entity as projection on db_schema.Entity4 excluding { Attribute4 } + + /* API to talk to other services. */ + action send4 ( + messageToPass: String + ) returns String; +} diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4withcds.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4withcds.js new file mode 100644 index 000000000..acb705f09 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4withcds.js @@ -0,0 +1,17 @@ +const cds = require("@sap/cds"); + +class TestService extends cds.ApplicationService { + init() { + this.before('READ', 'Test', (req) => req.reply([])) //req + this.after('READ', this.entities, req => req.target.data) //req + return super.init() + } +} +module.exports = TestService + +cds.serve('./test-service').with((srv) => { + const { Test, Service4 } = this.entities + srv.before('READ', 'Test', (req) => req.reply([])) //req + srv.on('READ', [Test, Service4], req => req.target.data) //req + srv.after('READ', req => req.target.data) //req +}) \ No newline at end of file diff --git a/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.expected b/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.expected index 75e44aa17..63a17349a 100644 --- a/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.expected +++ b/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.expected @@ -1,3 +1,5 @@ | requesthandler.js:5:43:5:45 | req | | requesthandler.js:6:34:6:36 | req | -| requesthandler.js:16:34:16:36 | req | +| requesthandler.js:10:28:10:30 | req | +| requesthandler.js:11:26:11:28 | req | +| requesthandler.js:18:34:18:36 | req | diff --git a/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js b/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js index b48a78185..da965176d 100644 --- a/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js +++ b/javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js @@ -7,6 +7,8 @@ class BooksService extends cds.ApplicationService { let [ID] = req.params return Object.assign(Books.data[ID], req.data) }) + this.after('READ', req => req.target.data) + this.before('*', req => req.target.data) return super.init() } }