Skip to content

Commit 375a26a

Browse files
committed
Mark the entire shortcut method call as primary location
The previous query marked only the base shortcut call (srv.read, srv.update, ...) as the primary location. This is a bit misleading since the "this query" part would mean the entire chained method call. Therefore, make the chained method call at the very end as the primary location, thereby making the entire chained call including the base shortcut call as the primary location.
1 parent d26d930 commit 375a26a

File tree

3 files changed

+145
-55
lines changed

3 files changed

+145
-55
lines changed

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

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -125,7 +125,10 @@ class StringConcatParameterOfCqlShortcutMethodCall extends CqlInjectionSink {
125125
this = cqlShortcutMethodCallWithStringConcat.getStringConcatParameter()
126126
}
127127

128-
override DataFlow::Node getQuery() { result = cqlShortcutMethodCallWithStringConcat }
128+
override DataFlow::Node getQuery() {
129+
result =
130+
cqlShortcutMethodCallWithStringConcat.(CqlShortcutMethodCall).getFinalChainedMethodCall()
131+
}
129132
}
130133

131134
/**

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

Lines changed: 20 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -589,7 +589,7 @@ class CdsUser extends API::Node {
589589
/**
590590
* A transaction object to be carried out by a service that it is initialized
591591
* with through a call to `tx`. Note that there are two types of styles when
592-
* it comes to using the transaction object, that is, within a callback or in a
592+
* it comes to using the transaction object, that is, within a callback or in a
593593
* `try`-`catch` block:
594594
*
595595
* ``` javascript
@@ -607,14 +607,14 @@ class CdsUser extends API::Node {
607607
* await tx.rollback(e);
608608
* }
609609
* ```
610-
*
610+
*
611611
* The former style allows for automatic transaction management by the framework
612612
* (notice there are no `commit` and `rollback` calls), while the latter style
613613
* makes the low-level transaction operations explicit.
614-
*
614+
*
615615
* To accommodate for both styles, this class captures both the transaction call
616616
* itself, and the parameter of the callback.
617-
*
617+
*
618618
* Note that the call to `tx` can optionally take a context object as its first
619619
* parameter that lets overriding of certain options such as `user`.
620620
*/
@@ -912,6 +912,22 @@ class CqlShortcutMethodCall extends CqlQueryRunnerCall {
912912
}
913913

914914
abstract override DataFlow::Node getAQueryParameter();
915+
916+
/**
917+
* Gets the final method call that is transitively chained on this method call. e.g.
918+
* given these fluent API calls on `srv.update`:
919+
* ``` javascript
920+
* srv.update(Entity1).set("col=col+1").where("col=" + id)
921+
* srv.update(Entity1).set("col=col+1").where`col=${id}`
922+
* srv.update(Entity1).set`col=col+1`.where("col=${id}")
923+
* srv.update(Entity1).set`col=col+1`.where`col=${id}`
924+
* ```
925+
* This predicate gets the entire `srv.update(Entity1).set....where...` call.
926+
*/
927+
DataFlow::CallNode getFinalChainedMethodCall() {
928+
result = this.getAChainedMethodCall(_) and
929+
not exists(result.asExpr().getParentExpr())
930+
}
915931
}
916932

917933
class CqlReadMethodCall extends CqlShortcutMethodCall {

0 commit comments

Comments
 (0)