Skip to content

Commit 7e37342

Browse files
committed
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
1 parent 4567df0 commit 7e37342

File tree

5 files changed

+95
-20
lines changed

5 files changed

+95
-20
lines changed

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/CDS.qll

Lines changed: 19 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -60,13 +60,14 @@ class CdsServeCall extends DataFlow::CallNode {
6060

6161
Expr getServiceRepresentation() { result = serviceRepresentation }
6262

63-
UserDefinedApplicationService getServiceDefinition() {
63+
ServiceInstance getServiceDefinition() {
6464
/* 1. The argument to cds.serve is "all" */
6565
this.getServiceRepresentation().getStringValue() = "all" and
6666
result = any(UserDefinedApplicationService service)
6767
or
6868
/* 2. The argument to cds.serve is a name of the service */
69-
result.getUnqualifiedName() = this.getServiceRepresentation().getStringValue()
69+
result.(UserDefinedApplicationService).getUnqualifiedName() =
70+
this.getServiceRepresentation().getStringValue()
7071
or
7172
/* 3. The argument to cds.serve is a name by which the service is required */
7273
exists(RequiredService requiredService |
@@ -217,27 +218,28 @@ class CdsServeWithCall extends MethodCallNode {
217218
*/
218219
class ServiceInstanceFromServeWithParameter extends ServiceInstance {
219220
CdsServeCall cdsServe;
221+
CdsServeWithCall cdsServeWith;
220222

221223
ServiceInstanceFromServeWithParameter() {
222-
exists(CdsServeWithCall cdsServeWith |
223-
/*
224-
* cds.serve('./srv/some-service1').with ((srv) => { // Parameter `srv` is captured.
225-
* srv.on ('READ','SomeEntity1', (req) => req.reply([...]))
226-
* })
227-
*/
224+
/*
225+
* cds.serve('./srv/some-service1').with ((srv) => { // Parameter `srv` is captured.
226+
* srv.on ('READ','SomeEntity1', (req) => req.reply([...]))
227+
* })
228+
*/
228229

229-
this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr)
230-
or
231-
/*
232-
* cds.serve('./srv/some-service2').with (function() { // Parameter `this` is captured.
233-
* this.on ('READ','SomeEntity2', (req) => req.reply([...]))
234-
* })
235-
*/
230+
this.(ThisNode).getBinder().asExpr() = cdsServeWith.getDecorator().(FunctionExpr)
231+
or
232+
/*
233+
* cds.serve('./srv/some-service2').with (function() { // Parameter `this` is captured.
234+
* this.on ('READ','SomeEntity2', (req) => req.reply([...]))
235+
* })
236+
*/
236237

237-
this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow()
238-
)
238+
this = cdsServeWith.getDecorator().(ArrowFunctionExpr).getParameter(0).flow()
239239
}
240240

241+
HandlerRegistration getAHandlerRegistration() { result = cdsServeWith.getAHandlerRegistration() }
242+
241243
override UserDefinedApplicationService getDefinition() {
242244
/* 1. The argument to cds.serve is "all" */
243245
cdsServe.getServiceRepresentation().getStringValue() = "all" and

javascript/frameworks/cap/lib/advanced_security/javascript/frameworks/cap/RemoteFlowSources.qll

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,8 @@ import advanced_security.javascript.frameworks.cap.CDS
1010
* SomeService.after("SomeEvent", "SomeEntity", (msg) => { ... });
1111
* ```
1212
* All the parameters named `req` and `msg` are captured in the above example.
13+
*
14+
* REQUIRES that a `UserDefinedApplicationService` is explicitly defined
1315
*/
1416
class HandlerParameter extends ParameterNode, RemoteFlowSource {
1517
Handler handler;
@@ -49,9 +51,11 @@ class HandlerParameter extends ParameterNode, RemoteFlowSource {
4951
* }
5052
* ```
5153
* parameters named `req` are captured in the above example.
54+
*
55+
* REQUIRES that a cds file has compiled AND that a service name is explicitly provided in the handler registration
5256
*/
53-
class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource {
54-
ServiceinCDSHandlerParameter() {
57+
class ServiceinCDSHandlerParameterWithName extends ParameterNode, RemoteFlowSource {
58+
ServiceinCDSHandlerParameterWithName() {
5559
exists(MethodCallNode m, CdlEntity entity, string entityName |
5660
entity.getName().regexpReplaceAll(".*\\.", "") = entityName and
5761
m.getArgument(1).asExpr().getStringValue().regexpReplaceAll("'", "") = entityName and
@@ -64,3 +68,46 @@ class ServiceinCDSHandlerParameter extends ParameterNode, RemoteFlowSource {
6468
result = "Parameter of an event handler belonging to an exposed service defined in a cds file"
6569
}
6670
}
71+
72+
/**
73+
* A parameter of a handler registered for a service on an event. e.g.
74+
* ```javascript
75+
* cds.serve('./test-service').with((srv) => {
76+
* srv.before('READ', '*', (req) => req.reply([]))
77+
* })
78+
* ```
79+
* The parameter named `req` is captured in the above example.
80+
*
81+
* DOES NOT REQUIRE that a `UserDefinedApplicationService` is explicitly defined
82+
* DOES NOT REQUIRE that the name is provided explicitly
83+
*/
84+
class HandlerParameterImplicitService extends ParameterNode, RemoteFlowSource {
85+
Handler handler;
86+
HandlerRegistration handlerRegistration;
87+
88+
HandlerParameterImplicitService() {
89+
exists(ServiceInstanceFromServeWithParameter service |
90+
handler = handlerRegistration.getHandler() and
91+
this = handler.getParameter(0) and
92+
service.getAHandlerRegistration() = handlerRegistration and
93+
//this will otherwise duplicate on the case where we do actually know the
94+
//name from the cds file and it matches up
95+
//only relevant if you are using the specific type anyhow (as opposed to RemoteFlowSource)
96+
not this instanceof ServiceinCDSHandlerParameterWithName
97+
)
98+
}
99+
100+
override string getSourceType() {
101+
result = "Parameter of an event handler belonging to an implicitly defined service"
102+
}
103+
104+
/**
105+
* Gets the handler this is a parameter of.
106+
*/
107+
Handler getHandler() { result = handler }
108+
109+
/**
110+
* Gets the handler registration registering the handler it is a parameter of.
111+
*/
112+
HandlerRegistration getHandlerRegistration() { result = handlerRegistration }
113+
}

javascript/frameworks/cap/test/models/cds/remoteflowsources/remoteflowsource.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ import javascript
22
import advanced_security.javascript.frameworks.cap.RemoteFlowSources
33

44
from RemoteFlowSource source
5-
select source
5+
select source
Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,24 @@
1+
//this service unit test is a replica of requesthandler.js
2+
const cds = require("@sap/cds");
3+
class BooksService extends cds.ApplicationService {
4+
init() {
5+
const { Books, Authors } = this.entities
6+
this.on('READ', [Books, Authors], req => req.target.data) //req
7+
this.on('UPDATE', Books, req => { //req
8+
let [ID] = req.params
9+
return Object.assign(Books.data[ID], req.data)
10+
})
11+
this.after('READ', req => req.target.data) //req
12+
this.before('*', req => req.target.data) //req
13+
return super.init()
14+
}
15+
}
16+
module.exports = BooksService
17+
18+
cds.serve('./test-service').with((srv) => {
19+
srv.before('READ', 'Books', (req) => req.reply([])) //req
20+
})
21+
22+
cds.serve('./test-service').with((srv) => {
23+
srv.before('READ', 'Test', (req) => req.reply([])) //req
24+
})

javascript/frameworks/cap/test/models/cds/requesthandler/requesthandler.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ class BooksService extends cds.ApplicationService {
77
let [ID] = req.params
88
return Object.assign(Books.data[ID], req.data)
99
})
10+
this.after('READ', req => req.target.data)
11+
this.before('*', req => req.target.data)
1012
return super.init()
1113
}
1214
}

0 commit comments

Comments
 (0)