Skip to content

Commit 440712f

Browse files
committed
Do some refactoring
1 parent 3b7d741 commit 440712f

File tree

3 files changed

+101
-34
lines changed

3 files changed

+101
-34
lines changed

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

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,6 @@ class TaintedClause instanceof CqlClause {
1818
Expr asExpr() { result = super.asExpr() }
1919
}
2020

21-
/**
22-
* Call to`cds.db.run`
23-
* or
24-
* an await surrounding a sql statement
25-
*/
26-
class CQLSink extends DataFlow::Node {
27-
CQLSink() {
28-
this = any(CdsFacade cds).getMember("db").getMember("run").getACall().getAnArgument()
29-
or
30-
exists(AwaitExpr a, CqlClause clause |
31-
a.getAChildExpr() = clause.asExpr() and this.asExpr() = clause.asExpr()
32-
)
33-
}
34-
}
35-
3621
/**
3722
* a more heurisitic based taint step
3823
* captures one of the alternative ways to construct query strings:
@@ -57,7 +42,14 @@ class CqlIConfiguration extends TaintTracking::Configuration {
5742

5843
override predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
5944

60-
override predicate isSink(DataFlow::Node sink) { sink instanceof CQLSink }
45+
override predicate isSink(DataFlow::Node node) {
46+
node = any(CdsFacade cds).getMember("db").getMember("run").getACall().getAnArgument()
47+
or
48+
exists(AwaitExpr awaitExpr, CqlClause clause |
49+
node.asExpr() = clause.asExpr() and
50+
awaitExpr.getOperand() = clause.asExpr()
51+
)
52+
}
6153

6254
override predicate isSanitizer(DataFlow::Node node) {
6355
super.isSanitizer(node) or

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

Lines changed: 75 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import advanced_security.javascript.frameworks.cap.CQL
77
import advanced_security.javascript.frameworks.cap.RemoteFlowSources
88

99
/**
10+
* The CDS facade that provides useful interfaces to the current CAP application.
1011
* ```js
1112
* const cds = require('@sap/cds')
1213
* ```
@@ -31,6 +32,23 @@ class CdsEntitiesCall extends DataFlow::CallNode {
3132
}
3233
}
3334

35+
/**
36+
* The property `db` of on a CDS facade, often accessed as `cds.db`.
37+
*/
38+
class CdsDb extends SourceNode {
39+
CdsDb() { exists(CdsFacade cds | this = cds.getMember("db").asSource()) }
40+
41+
MethodCallNode getRunCall() { result = this.getAMemberCall("run") }
42+
43+
MethodCallNode getCreateCall() { result = this.getAMemberCall("create") }
44+
45+
MethodCallNode getUpdateCall() { result = this.getAMemberCall("update") }
46+
47+
MethodCallNode getDeleteCall() { result = this.getAMemberCall("delete") }
48+
49+
MethodCallNode getInsertCall() { result = this.getAMemberCall("insert") }
50+
}
51+
3452
/**
3553
* An entity object that belongs to a service.
3654
*
@@ -123,7 +141,8 @@ class CdsConnectToCall extends DataFlow::CallNode {
123141
}
124142

125143
/**
126-
* A dataflow node that represents a service. Note that its definition is a `UserDefinedApplicationService`, not a `ServiceInstance`.
144+
* A dataflow node that represents a service.
145+
* Note that its definition is a `UserDefinedApplicationService`, not a `ServiceInstance`.
127146
*/
128147
abstract class ServiceInstance extends SourceNode {
129148
abstract UserDefinedApplicationService getDefinition();
@@ -764,5 +783,59 @@ class HandlerParameterData instanceof PropRead {
764783
)
765784
}
766785

767-
string toString() { result = PropRead.super.toString() }
786+
string toString() { result = super.toString() }
787+
}
788+
789+
/**
790+
* A call to a method capable of running a CQL query. This includes the following:
791+
* - Generic query runners: `cds.run`, `cds.db.run`, `srv.run`
792+
* - Shortcut to CQL's `READ`: `cds.read`, `cds.db.read`, `srv.read`
793+
* - Shortcut to CQL's `CREATE`: `cds.create`, `cds.db.create`, `srv.create`
794+
* - Shortcut to CQL's `INSERT`: `cds.insert`, `cds.db.insert`, `srv.insert`
795+
* - Shortcut to CQL's `UPSERT`: `cds.upsert`, `cds.db.upsert`, `srv.upsert`
796+
* - Shortcut to CQL's `UPDATE`: `cds.update`, `cds.db.update`, `srv.update`
797+
* - Shortcut to CQL's `DELETE`: `cds.delete`, `cds.db.delete`, `srv.delete`
798+
*/
799+
abstract class CqlQueryRunnerCall extends MethodCallNode {
800+
SourceNode base;
801+
string methodName;
802+
803+
CqlQueryRunnerCall() {
804+
this = base.getAMemberCall(methodName) and
805+
(
806+
/*
807+
* 1. Method call on the CDS facade or the base database service,
808+
* accessed as `cds.db`.
809+
*/
810+
811+
exists(CdsFacade cds | base = cds.asSource()) or
812+
exists(CdsDb cdsDb | base = cdsDb) or
813+
/* 2. Method call on a service instance object. */
814+
exists(ServiceInstance srv | base = srv)
815+
)
816+
}
817+
}
818+
819+
class CqlRunMethodCall extends CqlQueryRunnerCall {
820+
CqlRunMethodCall() { this.getMethodName() = "run" }
821+
}
822+
823+
class CqlReadMethodCall extends CqlQueryRunnerCall {
824+
CqlReadMethodCall() { this.getMethodName() = "read" }
825+
}
826+
827+
class CqlCreateMethodCall extends CqlQueryRunnerCall {
828+
CqlCreateMethodCall() { this.getMethodName() = "create" }
829+
}
830+
831+
class CqlUpdateMethodCall extends CqlQueryRunnerCall {
832+
CqlUpdateMethodCall() { this.getMethodName() = "update" }
833+
}
834+
835+
class CqlDeleteMethodCall extends CqlQueryRunnerCall {
836+
CqlDeleteMethodCall() { this.getMethodName() = "delete" }
837+
}
838+
839+
class CqlInsertMethodCall extends CqlQueryRunnerCall {
840+
CqlInsertMethodCall() { this.getMethodName() = "insert" }
768841
}

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

Lines changed: 18 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,14 @@ class CqlQueryBase extends VarRef {
1111
exists(string name |
1212
this.getName() = name and
1313
name in ["SELECT", "INSERT", "DELETE", "UPDATE", "UPSERT"] and
14-
/* Made available as a global variable */
15-
exists(GlobalVariable queryBase | this = queryBase.getAReference())
16-
or
17-
/* Imported from `cds.ql` */
18-
exists(CdsFacade cds |
19-
cds.getMember("ql").getMember(name).getAValueReachableFromSource().asExpr() = this
14+
(
15+
/* Made available as a global variable */
16+
exists(GlobalVariable queryBase | this = queryBase.getAReference())
17+
or
18+
/* Imported from `cds.ql` */
19+
exists(CdsFacade cds |
20+
cds.getMember("ql").getMember(name).getAValueReachableFromSource().asExpr() = this
21+
)
2022
)
2123
)
2224
}
@@ -88,22 +90,22 @@ Expr getRootReceiver(Expr e) {
8890
* provided by the module cds.ql
8991
*/
9092
private newtype TCqlClause =
91-
TaggedTemplate(TaggedTemplateExpr taggedTemplateExpr) {
93+
TTaggedTemplate(TaggedTemplateExpr taggedTemplateExpr) {
9294
exists(CqlQueryBase base | base = getRootReceiver(taggedTemplateExpr)) or
9395
exists(CqlQueryBaseCall call | call = getRootReceiver(taggedTemplateExpr))
9496
} or
95-
MethodCall(MethodCallExpr callExpr) {
97+
TMethodCall(MethodCallExpr callExpr) {
9698
exists(CqlQueryBase base | base = getRootReceiver(callExpr)) or
9799
exists(CqlQueryBaseCall call | call = getRootReceiver(callExpr))
98100
} or
99-
ShortcutCall(CqlQueryBaseCall callExpr)
101+
TShortcutCall(CqlQueryBaseCall callExpr)
100102

101103
class CqlClause extends TCqlClause {
102-
TaggedTemplateExpr asTaggedTemplate() { this = TaggedTemplate(result) }
104+
TaggedTemplateExpr asTaggedTemplate() { this = TTaggedTemplate(result) }
103105

104-
MethodCallExpr asMethodCall() { this = MethodCall(result) }
106+
MethodCallExpr asMethodCall() { this = TMethodCall(result) }
105107

106-
CallExpr asShortcutCall() { this = ShortcutCall(result) }
108+
CallExpr asShortcutCall() { this = TShortcutCall(result) }
107109

108110
Node flow() { result = this.asExpr().flow() }
109111

@@ -169,8 +171,8 @@ class CqlClause extends TCqlClause {
169171
CqlQueryBase getCqlBase() { result = getRootReceiver(this.asMethodCall()) }
170172

171173
CqlQueryBaseCall getCqlBaseCall() {
172-
result = getRootReceiver(this.asTaggedTemplate()).(CqlQueryBaseCall) or
173-
result = getRootReceiver(this.asMethodCall()).(CqlQueryBaseCall)
174+
result = getRootReceiver(this.asTaggedTemplate()) or
175+
result = getRootReceiver(this.asMethodCall())
174176
}
175177

176178
/** Describes a parent expression relation */
@@ -184,9 +186,9 @@ class CqlClause extends TCqlClause {
184186
* Possible cases for constructing a chain of clauses:
185187
*
186188
* (looking at the terminal clause and its possible parent types as tuples: (this, parent))
187-
* 1. MethodCall.MethodCall
189+
* 1. TMethodCall.TMethodCall
188190
* - example `(SELECT.from(Table), SELECT.from(Table).where("col1='*'"))`
189-
* 2. ShortcutCall.MethodCall
191+
* 2. TShortcutCall.TMethodCall
190192
* - example `(SELECT("col1, col2"), SELECT("col1, col2").from("Table"))`
191193
*
192194
* Note that ShortcutCalls cannot be added to any clause chain other than the first position, e.g.

0 commit comments

Comments
 (0)