Skip to content

Commit bb0fd86

Browse files
committed
Address bugs in UserProvidedPropertyReadOfHandlerParameterOfExposedService
- Extension to definitions: - Add an explicit case named `ExportedClosureApplicationServiceDefinition` of a `UserDefinedApplicationService` that is defined in the form of an exported function whose first parameter represents the service instance. - Add a definition `ServiceInstanceFromExportedClosureParameter` that matches above class. - Bug fixes: - Address a bug in the `UserDefinedApplicationService.isExposed/0` where a simple negation also captured cases where CDS declarations were missing. - Address a bug in the `HandlerParameterOfExposedService` where the trailing `.getCdsDeclaration()` was missing. NOTE: See issue #221 to learn more about the `HACK` block comment.
1 parent fe90260 commit bb0fd86

File tree

2 files changed

+52
-4
lines changed

2 files changed

+52
-4
lines changed

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

Lines changed: 49 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,12 +294,21 @@ class DbServiceInstanceFromCdsConnectTo extends ServiceInstanceFromCdsConnectTo,
294294
override UserDefinedApplicationService getDefinition() { none() }
295295
}
296296

297+
class ServiceInstanceFromExportedClosureParameter extends ServiceInstance {
298+
ExportedClosureApplicationServiceDefinition exportedClosure;
299+
300+
ServiceInstanceFromExportedClosureParameter() { this = exportedClosure.getParameter(0) }
301+
302+
override UserDefinedApplicationService getDefinition() { result = exportedClosure }
303+
}
304+
297305
/**
298306
* A call to `before`, `on`, or `after` on an `cds.ApplicationService`.
299307
* It registers an handler to be executed when an event is fired,
300308
* to do something with the incoming request or event as its parameter.
301309
*/
302310
class HandlerRegistration extends MethodCallNode {
311+
/** The instance of the service a handler is registered on. */
303312
ServiceInstance srv;
304313
string methodName;
305314

@@ -308,6 +317,9 @@ class HandlerRegistration extends MethodCallNode {
308317
methodName = ["before", "on", "after"]
309318
}
310319

320+
/**
321+
* Gets the instance of the service a handler is registered on.
322+
*/
311323
ServiceInstance getService() { result = srv }
312324

313325
/**
@@ -347,7 +359,7 @@ class HandlerRegistration extends MethodCallNode {
347359

348360
/**
349361
* The first parameter of a handler, representing the request object received either directly
350-
* from a user, or from another service that may be internal (defined in the same application)
362+
* from a user, or from another service that may be internal (defined in the same application)
351363
* or external (defined in another application, or even served from a different server).
352364
* e.g.
353365
* ``` javascript
@@ -356,7 +368,7 @@ class HandlerRegistration extends MethodCallNode {
356368
* this.before("SomeEvent", "SomeEntity", (req, next) => { ... });
357369
* this.after("SomeEvent", "SomeEntity", (req, next) => { ... });
358370
* }
359-
* ```
371+
* ```
360372
* All parameters named `req` above are captured. Also see `HandlerParameterOfExposedService`
361373
* for a subset of this class that is only about handlers exposed to some protocol.
362374
*/
@@ -506,7 +518,7 @@ abstract class UserDefinedApplicationService extends UserDefinedService {
506518
/**
507519
* Holds if this service supports access from the outside through any kind of protocol.
508520
*/
509-
predicate isExposed() { not this.isInternal() }
521+
predicate isExposed() { exists(this.getCdsDeclaration()) and not this.isInternal() }
510522

511523
/**
512524
* Holds if this service does not support access from the outside through any kind of protocol, thus being internal only.
@@ -554,6 +566,40 @@ class ImplMethodCallApplicationServiceDefinition extends MethodCallNode,
554566
override FunctionNode getInitFunction() { result = this.getArgument(0) }
555567
}
556568

569+
/**
570+
* A user-defined application service that comes in a form of an exported
571+
* closure. e.g. Given the below code,
572+
* ``` javascript
573+
* const cds = require("@sap/cds");
574+
*
575+
* module.exports = (srv) => {
576+
* srv.before("SomeEvent1", "SomeEntity", (req, res) => { ... })
577+
* srv.on("SomeEvent2", (req) => { ... } )
578+
* srv.after("SomeEvent3", (req) => { ... } )
579+
* }
580+
* ```
581+
* This class captures the entire `(srv) => { ... }` function that is
582+
* exported.
583+
*/
584+
class ExportedClosureApplicationServiceDefinition extends FunctionNode,
585+
UserDefinedApplicationService
586+
{
587+
ExportedClosureApplicationServiceDefinition() {
588+
/*
589+
* ==================== HACK ====================
590+
* See issue #221.
591+
*/
592+
593+
exists(PropWrite moduleExports |
594+
moduleExports.getBase().asExpr().(VarAccess).getName() = "module" and
595+
moduleExports.getPropertyName() = "exports" and
596+
this = moduleExports.getRhs()
597+
)
598+
}
599+
600+
override FunctionNode getInitFunction() { result = this }
601+
}
602+
557603
abstract class InterServiceCommunicationMethodCall extends MethodCallNode {
558604
string name;
559605
ServiceInstance recipient;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,9 @@ class HandlerParameterOfExposedService extends HandlerParameter {
3030
* is known.
3131
*/
3232

33-
not exists(this.getHandler().getHandlerRegistration().getService().getDefinition())
33+
not exists(
34+
this.getHandler().getHandlerRegistration().getService().getDefinition().getCdsDeclaration()
35+
)
3436
}
3537
}
3638

0 commit comments

Comments
 (0)