From 7e373427d393b85622fbfe3b76c3ac8157adcbcc Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 14 Jul 2025 16:51:31 -0400 Subject: [PATCH 1/8] Enhance remote flow sources for CAP add test cases to cover cap event handler registrations that were previously missing add remote flow source type to cover more cases of event handler registration --- .../javascript/frameworks/cap/CDS.qll | 36 ++++++------- .../frameworks/cap/RemoteFlowSources.qll | 51 ++++++++++++++++++- .../cds/remoteflowsources/remoteflowsource.ql | 2 +- .../remoteflowsources/srv/service3nocds.js | 24 +++++++++ .../cds/requesthandler/requesthandler.js | 2 + 5 files changed, 95 insertions(+), 20 deletions(-) create mode 100644 javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js 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..b62da91eb 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 @@ -60,13 +60,14 @@ class CdsServeCall extends DataFlow::CallNode { Expr getServiceRepresentation() { result = serviceRepresentation } - UserDefinedApplicationService getServiceDefinition() { + ServiceInstance getServiceDefinition() { /* 1. The argument to cds.serve is "all" */ this.getServiceRepresentation().getStringValue() = "all" and result = any(UserDefinedApplicationService service) or /* 2. The argument to cds.serve is a name of the service */ - result.getUnqualifiedName() = this.getServiceRepresentation().getStringValue() + result.(UserDefinedApplicationService).getUnqualifiedName() = + this.getServiceRepresentation().getStringValue() or /* 3. The argument to cds.serve is a name by which the service is required */ exists(RequiredService requiredService | @@ -217,27 +218,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 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..bd2305367 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 @@ -10,6 +10,8 @@ import advanced_security.javascript.frameworks.cap.CDS * SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... }); * ``` * All the parameters named `req` and `msg` are captured in the above example. + * + * REQUIRES that a `UserDefinedApplicationService` is explicitly defined */ class HandlerParameter extends ParameterNode, RemoteFlowSource { Handler handler; @@ -49,9 +51,11 @@ class HandlerParameter extends ParameterNode, RemoteFlowSource { * } * ``` * parameters named `req` are captured in the above example. + * + * REQUIRES that a cds file has compiled AND that a service name is explicitly provided in the handler registration */ -class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource { - ServiceinCDSHandlerParameter() { +class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSource { + ServiceinCDSHandlerParameterWithName() { exists(MethodCallNode m, CdlEntity entity, string entityName | entity.getName().regexpReplaceAll(".*\\.", "") = entityName and m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName and @@ -64,3 +68,46 @@ class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource { result = "Parameter of an event handler belonging to an exposed service defined in a cds file" } } + +/** + * A parameter of a handler registered for a service on an event. e.g. + * ```javascript + * cds.serve('./test-service').with((srv) => { + * srv.before('READ', '*', (req) => req.reply([])) + * }) + * ``` + * The parameter named `req` is captured in the above example. + * + * DOES NOT REQUIRE that a `UserDefinedApplicationService` is explicitly defined + * DOES NOT REQUIRE that the name is provided explicitly + */ +class HandlerParameterImplicitService extends ParameterNode, RemoteFlowSource { + Handler handler; + HandlerRegistration handlerRegistration; + + HandlerParameterImplicitService() { + exists(ServiceInstanceFromServeWithParameter service | + handler = handlerRegistration.getHandler() and + this = handler.getParameter(0) and + service.getAHandlerRegistration() = handlerRegistration and + //this will otherwise duplicate on the case where we do actually know the + //name from the cds file and it matches up + //only relevant if you are using the specific type anyhow (as opposed to RemoteFlowSource) + not this instanceof ServiceinCDSHandlerParameterWithName + ) + } + + override string getSourceType() { + result = "Parameter of an event handler belonging to an implicitly defined 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 } +} 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..ec93a94c2 --- /dev/null +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js @@ -0,0 +1,24 @@ +//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 +}) + +cds.serve('./test-service').with((srv) => { + srv.before('READ', 'Test', (req) => req.reply([])) //req +}) \ No newline at end of file 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() } } From 90457ae4f878bc7e583865de3e1fd39e15309ab8 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Mon, 14 Jul 2025 17:10:56 -0400 Subject: [PATCH 2/8] Add adjusted expected files, forgot from prev commit --- .../test/models/cds/requesthandler/requesthandler.expected | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) 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 | From bb48b72d0b678cc186ccf1ba35f3bece88f0073d Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 15 Jul 2025 16:19:43 -0400 Subject: [PATCH 3/8] Revert accidental type change in CDS.qll --- .../lib/advanced_security/javascript/frameworks/cap/CDS.qll | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) 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 b62da91eb..0462dd72d 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 @@ -60,14 +60,13 @@ class CdsServeCall extends DataFlow::CallNode { Expr getServiceRepresentation() { result = serviceRepresentation } - ServiceInstance getServiceDefinition() { + UserDefinedApplicationService getServiceDefinition() { /* 1. The argument to cds.serve is "all" */ this.getServiceRepresentation().getStringValue() = "all" and result = any(UserDefinedApplicationService service) or /* 2. The argument to cds.serve is a name of the service */ - result.(UserDefinedApplicationService).getUnqualifiedName() = - this.getServiceRepresentation().getStringValue() + result.getUnqualifiedName() = this.getServiceRepresentation().getStringValue() or /* 3. The argument to cds.serve is a name by which the service is required */ exists(RequiredService requiredService | From 8658335fd5b309ad237476522741cd5e06ac43a7 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 15 Jul 2025 16:57:57 -0400 Subject: [PATCH 4/8] Apply review suggestions --- .../frameworks/cap/RemoteFlowSources.qll | 34 ++++++++++++++----- 1 file changed, 26 insertions(+), 8 deletions(-) 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 bd2305367..8ff11b93d 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 @@ -11,7 +11,7 @@ import advanced_security.javascript.frameworks.cap.CDS * ``` * All the parameters named `req` and `msg` are captured in the above example. * - * REQUIRES that a `UserDefinedApplicationService` is explicitly defined + * This REQUIRES that a `UserDefinedApplicationService` is explicitly defined. */ class HandlerParameter extends ParameterNode, RemoteFlowSource { Handler handler; @@ -52,7 +52,8 @@ class HandlerParameter extends ParameterNode, RemoteFlowSource { * ``` * parameters named `req` are captured in the above example. * - * REQUIRES that a cds file has compiled AND that a service name is explicitly provided in the handler registration + * This REQUIRES that a CDS file has successfully compiled + * AND that a service name is explicitly provided in the handler registration. */ class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSource { ServiceinCDSHandlerParameterWithName() { @@ -73,13 +74,13 @@ class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSour * A parameter of a handler registered for a service on an event. e.g. * ```javascript * cds.serve('./test-service').with((srv) => { - * srv.before('READ', '*', (req) => req.reply([])) + * srv.before('READ', '*', (req) => req.reply([])) * }) * ``` * The parameter named `req` is captured in the above example. * - * DOES NOT REQUIRE that a `UserDefinedApplicationService` is explicitly defined - * DOES NOT REQUIRE that the name is provided explicitly + * This DOES NOT REQUIRE that a `UserDefinedApplicationService` is explicitly defined and + * this also DOES NOT REQUIRE that the name is provided explicitly. */ class HandlerParameterImplicitService extends ParameterNode, RemoteFlowSource { Handler handler; @@ -90,9 +91,26 @@ class HandlerParameterImplicitService extends ParameterNode, RemoteFlowSource { handler = handlerRegistration.getHandler() and this = handler.getParameter(0) and service.getAHandlerRegistration() = handlerRegistration and - //this will otherwise duplicate on the case where we do actually know the - //name from the cds file and it matches up - //only relevant if you are using the specific type anyhow (as opposed to RemoteFlowSource) + /* + * this will otherwise duplicate on the case where we do actually know the + * name from the cds file and it matches up + * example: + * ``` + * srv.before('READ', 'Service1', (req) => req.reply([])) + * ``` + * where Service1 is also defined in: + * Service1.cds + * ``` + * { + * "namespace": "sap.capire.test", + * "definitions": { + * "sap.capire.test.Test": { + * "kind": "entity", + * ... + * ``` + * only relevant if you are using the specific type anyhow (as opposed to RemoteFlowSource) + */ + not this instanceof ServiceinCDSHandlerParameterWithName ) } From 71cabdd99123b882f05655e995ea04b2a41db67d Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Tue, 15 Jul 2025 16:58:15 -0400 Subject: [PATCH 5/8] Add missing expected file --- .../models/cds/remoteflowsources/remoteflowsource.expected | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected index 0b94e0bba..8009f62bc 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected @@ -1,2 +1,8 @@ | 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/service3nocds.js:23:33:23:35 | req | From f9ea162250d18ad0c5992650ddb2cc84d94aaa05 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Wed, 16 Jul 2025 11:21:30 -0400 Subject: [PATCH 6/8] Address review feedback RemoteFlowSources types add more testcases for all entity spec scenarios add extra ability to know name in one case for ServiceinCDSHandlerParameterWithName --- .../frameworks/cap/RemoteFlowSources.qll | 6 +++++- .../models/cds/remoteflowsources/db/schema.cds | 4 ++++ .../remoteflowsources/remoteflowsource.expected | 6 +++++- .../cds/remoteflowsources/srv/service3nocds.js | 4 ---- .../cds/remoteflowsources/srv/service4.cds | 11 +++++++++++ .../remoteflowsources/srv/service4withcds.js | 17 +++++++++++++++++ 6 files changed, 42 insertions(+), 6 deletions(-) create mode 100644 javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4.cds create mode 100644 javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service4withcds.js 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 8ff11b93d..063c2fe4a 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 @@ -59,7 +59,11 @@ class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSour ServiceinCDSHandlerParameterWithName() { exists(MethodCallNode m, CdlEntity entity, string entityName | entity.getName().regexpReplaceAll(".*\\.", "") = entityName and - m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName and + ( + m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName + or + m.getArgument(1).asExpr().(ArrayExpr).getAnElement().toString() = entityName + ) and this = m.getArgument(m.getNumArgument() - 1).(FunctionNode).getParameter(0) and m.getMethodName() in ["on", "before", "after"] ) 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 8009f62bc..dcc8f9d60 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.expected @@ -5,4 +5,8 @@ | 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/service3nocds.js:23:33:23:35 | 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/srv/service3nocds.js b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js index ec93a94c2..e44cb18d3 100644 --- a/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js +++ b/javascript/frameworks/cap/test/models/cds/remoteflowsources/srv/service3nocds.js @@ -17,8 +17,4 @@ module.exports = BooksService cds.serve('./test-service').with((srv) => { srv.before('READ', 'Books', (req) => req.reply([])) //req -}) - -cds.serve('./test-service').with((srv) => { - srv.before('READ', 'Test', (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 From 9912a3779e6ba8832c047f42b938efa3d63ea94c Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 17 Jul 2025 11:35:54 -0400 Subject: [PATCH 7/8] Apply changes from review sync refactor remoteflowsources --- .../javascript/frameworks/cap/CDS.qll | 25 +++- .../frameworks/cap/RemoteFlowSources.qll | 137 ++---------------- 2 files changed, 33 insertions(+), 129 deletions(-) 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 0462dd72d..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 @@ -345,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`. @@ -352,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. */ @@ -376,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 { @@ -823,7 +832,7 @@ class HandlerParameterData instanceof PropRead { string dataName; HandlerParameterData() { - this = handlerParameter.getAPropertyRead("data").getAPropertyRead(dataName) + this = handlerParameter.(SourceNode).getAPropertyRead("data").getAPropertyRead(dataName) } /** @@ -842,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 063c2fe4a..885d04db1 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,134 +2,29 @@ import javascript import advanced_security.javascript.frameworks.cap.CDS /** - * A parameter of a handler registered for a service on an event. e.g. - * ```javascript - * this.on("SomeEvent", "SomeEntity", (req) => { ... }); - * this.before("SomeEvent", "SomeEntity", (req, next) => { ... }); // only `req` is captured - * SomeService.on("SomeEvent", "SomeEntity", (msg) => { ... }); - * SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... }); + * Either a service is known and is exposed + * or + * there is a handler parameter that is not connected to a service + * possibly due to cds compilation failure + * or non explicit service references in source + * for example: * ``` - * All the parameters named `req` and `msg` are captured in the above example. - * - * This REQUIRES that a `UserDefinedApplicationService` is explicitly defined. - */ -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: - * ```javascript - * module.exports = srv => { - * srv.before('CREATE', 'Media', req => { // an entity name is used to describe which to register this handler to. - * ... - * }); - * } - * ``` - * parameters named `req` are captured in the above example. - * - * This REQUIRES that a CDS file has successfully compiled - * AND that a service name is explicitly provided in the handler registration. - */ -class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSource { - ServiceinCDSHandlerParameterWithName() { - exists(MethodCallNode m, CdlEntity entity, string entityName | - entity.getName().regexpReplaceAll(".*\\.", "") = entityName and - ( - m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName - or - m.getArgument(1).asExpr().(ArrayExpr).getAnElement().toString() = entityName - ) and - this = m.getArgument(m.getNumArgument() - 1).(FunctionNode).getParameter(0) and - m.getMethodName() in ["on", "before", "after"] - ) - } - - override string getSourceType() { - result = "Parameter of an event handler belonging to an exposed service defined in a cds file" - } -} - -/** - * A parameter of a handler registered for a service on an event. e.g. - * ```javascript * cds.serve('./test-service').with((srv) => { - * srv.before('READ', '*', (req) => req.reply([])) + * srv.after('READ', req => req.target.data) //req * }) * ``` - * The parameter named `req` is captured in the above example. - * - * This DOES NOT REQUIRE that a `UserDefinedApplicationService` is explicitly defined and - * this also DOES NOT REQUIRE that the name is provided explicitly. */ -class HandlerParameterImplicitService extends ParameterNode, RemoteFlowSource { - Handler handler; - HandlerRegistration handlerRegistration; - - HandlerParameterImplicitService() { - exists(ServiceInstanceFromServeWithParameter service | - handler = handlerRegistration.getHandler() and - this = handler.getParameter(0) and - service.getAHandlerRegistration() = handlerRegistration and - /* - * this will otherwise duplicate on the case where we do actually know the - * name from the cds file and it matches up - * example: - * ``` - * srv.before('READ', 'Service1', (req) => req.reply([])) - * ``` - * where Service1 is also defined in: - * Service1.cds - * ``` - * { - * "namespace": "sap.capire.test", - * "definitions": { - * "sap.capire.test.Test": { - * "kind": "entity", - * ... - * ``` - * only relevant if you are using the specific type anyhow (as opposed to RemoteFlowSource) - */ - - not this instanceof ServiceinCDSHandlerParameterWithName - ) +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 implicitly defined service" + 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 } } From 22fb705df75b343c6fb1fe99c59a01334819d1b4 Mon Sep 17 00:00:00 2001 From: Kristen Newbury Date: Thu, 17 Jul 2025 13:43:53 -0400 Subject: [PATCH 8/8] Add docs remoteflowsource cap --- .../frameworks/cap/RemoteFlowSources.qll | 18 ++++++++++++------ 1 file changed, 12 insertions(+), 6 deletions(-) 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 885d04db1..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,13 +2,19 @@ import javascript import advanced_security.javascript.frameworks.cap.CDS /** - * Either a service is known and is exposed - * or - * there is a handler parameter that is not connected to a service - * possibly due to cds compilation failure - * or non explicit service references in source - * for example: + * 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) => { ... }); + * SomeService.on("SomeEvent", "SomeEntity", (msg) => { ... }); + * SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... }); * ``` + * 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 * cds.serve('./test-service').with((srv) => { * srv.after('READ', req => req.target.data) //req * })