Skip to content

Commit 73a9c5b

Browse files
committed
Merge branch 'knewbury01/cds-util-path-traversal-query' of https://github.com/advanced-security/codeql-sap-js into knewbury01/cds-util-path-traversal-query
2 parents f35d885 + 234884e commit 73a9c5b

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

43 files changed

+621
-270
lines changed
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
---
22
library: true
33
name: advanced-security/javascript-sap-cap-models
4-
version: 2.0.0
4+
version: 2.1.0
55
extensionTargets:
66
codeql/javascript-all: "^2.4.0"

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

Lines changed: 22 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,10 @@ class CdsLogger extends MethodCallNode {
2222
string getName() { result = name }
2323
}
2424

25+
/**
26+
* A template literal that is not interpolated. It is basically a string literal
27+
* defined in backticks (\`\`).
28+
*/
2529
class ConstantOnlyTemplateLiteral extends TemplateLiteral {
2630
ConstantOnlyTemplateLiteral() {
2731
forall(Expr e | e = this.getAnElement() | e instanceof TemplateElement)
@@ -51,9 +55,26 @@ module CAPLogInjectionConfiguration implements DataFlow::ConfigSig {
5155
}
5256

5357
predicate isBarrier(DataFlow::Node node) {
58+
/*
59+
* This predicate includes cases such as:
60+
* 1. A CDS entity element lacking a type annotation.
61+
* - Possibly because it relies on a common aspect.
62+
* 2. A CDS entity element annotated with a non-string type listed above.
63+
*
64+
* Therefore, the data held by the handler parameter data (e.g. `req.data.X`)
65+
* has to be EXPLICITLY annotated as `String` or `LargeString` to be excluded
66+
* from the next condition.
67+
*/
68+
5469
exists(HandlerParameterData handlerParameterData |
5570
node = handlerParameterData and
56-
not handlerParameterData.getType() = ["cds.String", "cds.LargeString"]
71+
/* Note the use of `.. != ..` instead of `not .. = ..` below. */
72+
exists(string handlerParameterDataType |
73+
handlerParameterDataType = handlerParameterData.getType()
74+
|
75+
handlerParameterDataType != "cds.String" and
76+
handlerParameterDataType != "cds.LargeString"
77+
)
5778
)
5879
}
5980

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

Lines changed: 122 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -177,7 +177,27 @@ class ServiceInstanceFromConstructor extends ServiceInstance {
177177
}
178178

179179
/**
180-
* A read to `this` variable which represents the service whose definition encloses this variable access.
180+
* A read to `this` variable which represents the service whose definition encloses
181+
* this variable access.
182+
* e.g.1. Given this code:
183+
* ``` javascript
184+
* const cds = require("@sap/cds");
185+
* module.exports = class SomeService extends cds.ApplicationService {
186+
* init() {
187+
* this.on("SomeEvent", (req) => { ... } )
188+
* }
189+
* }
190+
* ```
191+
* This class captures the access to the `this` variable as in `this.on(...)`.
192+
*
193+
* e.g.2. Given this code:
194+
* ``` javascript
195+
* const cds = require('@sap/cds');
196+
* module.exports = cds.service.impl (function() {
197+
* this.on("SomeEvent", (req) => { ... })
198+
* })
199+
* ```
200+
* This class captures the access to the `this` variable as in `this.on(...)`.
181201
*/
182202
class ServiceInstanceFromThisNode extends ServiceInstance, ThisNode {
183203
UserDefinedApplicationService userDefinedApplicationService;
@@ -294,12 +314,56 @@ class DbServiceInstanceFromCdsConnectTo extends ServiceInstanceFromCdsConnectTo,
294314
override UserDefinedApplicationService getDefinition() { none() }
295315
}
296316

317+
/**
318+
* The 0-th parameter of an exported closure that represents the service being implemented. e.g.
319+
* ``` javascript
320+
* const cds = require('@sap/cds')
321+
* module.exports = (srv) => {
322+
* srv.on("SomeEvent1", (req) => { ... })
323+
* }
324+
* ```
325+
* This class captures the `srv` parameter of the exported arrow function. Also see
326+
* `ServiceInstanceFromImplMethodCallClosureParameter` which is similar to this.
327+
*/
328+
class ServiceInstanceFromExportedClosureParameter extends ServiceInstance, ParameterNode {
329+
ExportedClosureApplicationServiceDefinition exportedClosure;
330+
331+
ServiceInstanceFromExportedClosureParameter() { this = exportedClosure.getParameter(0) }
332+
333+
override UserDefinedApplicationService getDefinition() { result = exportedClosure }
334+
}
335+
336+
/**
337+
* The 0-th parameter of a callback (usually an arrow function) passed to `cds.service.impl` that
338+
* represents the service being implemented. e.g.
339+
* ``` javascript
340+
* const cds = require('@sap/cds')
341+
* module.exports = cds.service.impl((srv) => {
342+
* srv.on("SomeEvent1", (req) => { ... })
343+
* })
344+
* ```
345+
* This class captures the `srv` parameter of the exported arrow function. Also see
346+
* `ServiceInstanceFromExportedClosureParameter` which is similar to this.
347+
*/
348+
class ServiceInstanceFromImplMethodCallClosureParameter extends ServiceInstance, ParameterNode {
349+
ImplMethodCallApplicationServiceDefinition implMethodCallApplicationServiceDefinition;
350+
351+
ServiceInstanceFromImplMethodCallClosureParameter() {
352+
this = implMethodCallApplicationServiceDefinition.getInitFunction().getParameter(0)
353+
}
354+
355+
override UserDefinedApplicationService getDefinition() {
356+
result = implMethodCallApplicationServiceDefinition
357+
}
358+
}
359+
297360
/**
298361
* A call to `before`, `on`, or `after` on an `cds.ApplicationService`.
299362
* It registers an handler to be executed when an event is fired,
300363
* to do something with the incoming request or event as its parameter.
301364
*/
302365
class HandlerRegistration extends MethodCallNode {
366+
/** The instance of the service a handler is registered on. */
303367
ServiceInstance srv;
304368
string methodName;
305369

@@ -308,6 +372,9 @@ class HandlerRegistration extends MethodCallNode {
308372
methodName = ["before", "on", "after"]
309373
}
310374

375+
/**
376+
* Gets the instance of the service a handler is registered on.
377+
*/
311378
ServiceInstance getService() { result = srv }
312379

313380
/**
@@ -347,7 +414,7 @@ class HandlerRegistration extends MethodCallNode {
347414

348415
/**
349416
* 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)
417+
* from a user, or from another service that may be internal (defined in the same application)
351418
* or external (defined in another application, or even served from a different server).
352419
* e.g.
353420
* ``` javascript
@@ -356,14 +423,15 @@ class HandlerRegistration extends MethodCallNode {
356423
* this.before("SomeEvent", "SomeEntity", (req, next) => { ... });
357424
* this.after("SomeEvent", "SomeEntity", (req, next) => { ... });
358425
* }
359-
* ```
360-
* All parameters named `req` above are captured. Also see `HandlerParameterOfExposedService`
426+
* ```
427+
* All parameters named `req` above are captured. Also see
428+
* `RemoteflowSources::HandlerParameterOfExposedService`
361429
* for a subset of this class that is only about handlers exposed to some protocol.
362430
*/
363431
class HandlerParameter extends ParameterNode {
364432
Handler handler;
365433

366-
HandlerParameter() { this = handler.getParameter(0) }
434+
HandlerParameter() { this = isHandlerParameter(handler) }
367435

368436
Handler getHandler() { result = handler }
369437
}
@@ -506,7 +574,7 @@ abstract class UserDefinedApplicationService extends UserDefinedService {
506574
/**
507575
* Holds if this service supports access from the outside through any kind of protocol.
508576
*/
509-
predicate isExposed() { not this.isInternal() }
577+
predicate isExposed() { exists(this.getCdsDeclaration()) and not this.isInternal() }
510578

511579
/**
512580
* Holds if this service does not support access from the outside through any kind of protocol, thus being internal only.
@@ -539,9 +607,21 @@ class ES6ApplicationServiceDefinition extends ClassNode, UserDefinedApplicationS
539607

540608
/**
541609
* Subclassing `cds.ApplicationService` via a call to `cds.service.impl`.
542-
* ```js
610+
* e.g.1. Given this code:
611+
* ``` javascript
612+
* const cds = require('@sap/cds')
613+
* module.exports = cds.service.impl (function() {
614+
* this.on("SomeEvent1", (req) => { ... })
615+
* })
616+
* ```
617+
* This class captures the call `cds.service.impl (function() { ... })`.
618+
*
619+
* e.g.2. Given this code:
620+
* ``` javascript
543621
* const cds = require('@sap/cds')
544-
* module.exports = cds.service.impl (function() { ... })
622+
* module.exports = cds.service.impl ((srv) => {
623+
* srv.on("SomeEvent1", (req) => { ... })
624+
* })
545625
* ```
546626
*/
547627
class ImplMethodCallApplicationServiceDefinition extends MethodCallNode,
@@ -554,6 +634,40 @@ class ImplMethodCallApplicationServiceDefinition extends MethodCallNode,
554634
override FunctionNode getInitFunction() { result = this.getArgument(0) }
555635
}
556636

637+
/**
638+
* A user-defined application service that comes in a form of an exported
639+
* closure. e.g. Given the below code,
640+
* ``` javascript
641+
* const cds = require("@sap/cds");
642+
*
643+
* module.exports = (srv) => {
644+
* srv.before("SomeEvent1", "SomeEntity", (req, res) => { ... })
645+
* srv.on("SomeEvent2", (req) => { ... } )
646+
* srv.after("SomeEvent3", (req) => { ... } )
647+
* }
648+
* ```
649+
* This class captures the entire `(srv) => { ... }` function that is
650+
* exported.
651+
*/
652+
class ExportedClosureApplicationServiceDefinition extends FunctionNode,
653+
UserDefinedApplicationService
654+
{
655+
ExportedClosureApplicationServiceDefinition() {
656+
/*
657+
* ==================== HACK ====================
658+
* See issue #221.
659+
*/
660+
661+
exists(PropWrite moduleExports |
662+
moduleExports.getBase().asExpr().(VarAccess).getName() = "module" and
663+
moduleExports.getPropertyName() = "exports" and
664+
this = moduleExports.getRhs()
665+
)
666+
}
667+
668+
override FunctionNode getInitFunction() { result = this }
669+
}
670+
557671
abstract class InterServiceCommunicationMethodCall extends MethodCallNode {
558672
string name;
559673
ServiceInstance recipient;

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

Lines changed: 6 additions & 2 deletions
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

@@ -54,7 +56,9 @@ class UserProvidedPropertyReadOfHandlerParameterOfExposedService extends RemoteF
5456

5557
UserProvidedPropertyReadOfHandlerParameterOfExposedService() {
5658
/* 1. `req.(data|params|headers|id)` */
57-
this = handlerParameterOfExposedService.getAPropertyRead(["data", "params", "headers", "id"])
59+
this =
60+
handlerParameterOfExposedService
61+
.getAPropertyRead(["data", "params", "headers", "id", "_queryOptions"])
5862
or
5963
/* 2. APIs stemming from `req.http.req`: Defined by Express.js */
6064
exists(PropRead reqHttpReq |

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

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -39,3 +39,12 @@ private SourceNode cdsApplicationServiceInstantiation(TypeTracker t) {
3939
SourceNode cdsApplicationServiceInstantiation() {
4040
result = cdsApplicationServiceInstantiation(TypeTracker::end())
4141
}
42+
43+
private SourceNode isHandlerParameter(TypeTracker t, Handler handler) {
44+
result = handler.getParameter(0) or
45+
exists(TypeTracker t2 | result = isHandlerParameter(t, handler).track(t2, t))
46+
}
47+
48+
SourceNode isHandlerParameter(Handler handler) {
49+
result = isHandlerParameter(TypeTracker::end(), handler)
50+
}

javascript/frameworks/cap/lib/codeql-pack.lock.yml

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,27 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/concepts:
5-
version: 0.0.2
5+
version: 0.0.3
66
codeql/dataflow:
7-
version: 2.0.12
7+
version: 2.0.13
88
codeql/javascript-all:
9-
version: 2.6.8
9+
version: 2.6.9
1010
codeql/mad:
11-
version: 1.0.28
11+
version: 1.0.29
1212
codeql/regex:
13-
version: 1.0.28
13+
version: 1.0.29
1414
codeql/ssa:
15-
version: 2.0.4
15+
version: 2.0.5
1616
codeql/threat-models:
17-
version: 1.0.28
17+
version: 1.0.29
1818
codeql/tutorial:
19-
version: 1.0.28
19+
version: 1.0.29
2020
codeql/typetracking:
21-
version: 2.0.12
21+
version: 2.0.13
2222
codeql/util:
23-
version: 2.0.15
23+
version: 2.0.16
2424
codeql/xml:
25-
version: 1.0.28
25+
version: 1.0.29
2626
codeql/yaml:
27-
version: 1.0.28
27+
version: 1.0.29
2828
compiled: false

javascript/frameworks/cap/lib/qlpack.yml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
---
22
library: true
33
name: advanced-security/javascript-sap-cap-all
4-
version: 2.0.0
4+
version: 2.1.0
55
suites: codeql-suites
66
extractor: javascript
77
dependencies:

javascript/frameworks/cap/src/codeql-pack.lock.yml

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -2,27 +2,27 @@
22
lockVersion: 1.0.0
33
dependencies:
44
codeql/concepts:
5-
version: 0.0.2
5+
version: 0.0.3
66
codeql/dataflow:
7-
version: 2.0.12
7+
version: 2.0.13
88
codeql/javascript-all:
9-
version: 2.6.8
9+
version: 2.6.9
1010
codeql/mad:
11-
version: 1.0.28
11+
version: 1.0.29
1212
codeql/regex:
13-
version: 1.0.28
13+
version: 1.0.29
1414
codeql/ssa:
15-
version: 2.0.4
15+
version: 2.0.5
1616
codeql/threat-models:
17-
version: 1.0.28
17+
version: 1.0.29
1818
codeql/tutorial:
19-
version: 1.0.28
19+
version: 1.0.29
2020
codeql/typetracking:
21-
version: 2.0.12
21+
version: 2.0.13
2222
codeql/util:
23-
version: 2.0.15
23+
version: 2.0.16
2424
codeql/xml:
25-
version: 1.0.28
25+
version: 1.0.29
2626
codeql/yaml:
27-
version: 1.0.28
27+
version: 1.0.29
2828
compiled: false
Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
---
22
library: false
33
name: advanced-security/javascript-sap-cap-queries
4-
version: 2.0.0
4+
version: 2.1.0
55
suites: codeql-suites
66
extractor: javascript
77
dependencies:
88
codeql/javascript-all: "^2.4.0"
9-
advanced-security/javascript-sap-cap-models: "^2.0.0"
10-
advanced-security/javascript-sap-cap-all: "^2.0.0"
9+
advanced-security/javascript-sap-cap-models: "^2.1.0"
10+
advanced-security/javascript-sap-cap-all: "^2.1.0"
1111
default-suite-file: codeql-suites/javascript-code-scanning.qls

0 commit comments

Comments
 (0)