Skip to content

Commit e5d6599

Browse files
committed
Python: Use DefinitionNode instead of Assign
Based on github#6155 (comment): > Hmm... Would it be better to do this using DefinitionNode instead of > Assign? The latter is fairly limited in what it can represent, and also > raises questions of whether this definition is sound with regard to > control-flow splitting.
1 parent 94bcda3 commit e5d6599

File tree

2 files changed

+18
-18
lines changed

2 files changed

+18
-18
lines changed

python/ql/src/semmle/python/frameworks/Aiohttp.qll

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -645,16 +645,14 @@ module AiohttpWebModel {
645645
DataFlow::Node value;
646646

647647
AiohttpResponseCookieSubscriptWrite() {
648-
exists(Assign assign, Subscript subscript |
649-
// Since there is no `DataFlow::Node` for the assign (since it's a statement,
650-
// and not an expression) there doesn't seem to be any _good_ choice for `this`,
651-
// so just picking the whole subscript...
652-
this.asExpr() = subscript
648+
exists(SubscriptNode subscript |
649+
// To give `this` a value, we need to choose between either LHS or RHS,
650+
// and just go with the LHS
651+
this.asCfgNode() = subscript
653652
|
654-
assign.getATarget() = subscript and
655-
subscript.getObject() = aiohttpResponseInstance().getMember("cookies").getAUse().asExpr() and
656-
index.asExpr() = subscript.getIndex() and
657-
value.asExpr() = assign.getValue()
653+
subscript.getObject() = aiohttpResponseInstance().getMember("cookies").getAUse().asCfgNode() and
654+
value.asCfgNode() = subscript.(DefinitionNode).getValue() and
655+
index.asCfgNode() = subscript.getIndex()
658656
)
659657
}
660658

python/ql/src/semmle/python/frameworks/Django.qll

Lines changed: 11 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1412,18 +1412,20 @@ private module PrivateDjango {
14121412
DataFlow::Node value;
14131413

14141414
DjangoResponseCookieSubscriptWrite() {
1415-
exists(Assign assign, Subscript subscript, DataFlow::AttrRead cookieLookup |
1416-
// Since there is no `DataFlow::Node` for the assign (since it's a statement,
1417-
// and not an expression) there doesn't seem to be any _good_ choice for `this`,
1418-
// so just picking the whole subscript...
1419-
this.asExpr() = subscript
1415+
exists(SubscriptNode subscript, DataFlow::AttrRead cookieLookup |
1416+
// To give `this` a value, we need to choose between either LHS or RHS,
1417+
// and just go with the LHS
1418+
this.asCfgNode() = subscript
14201419
|
14211420
cookieLookup.getAttributeName() = "cookies" and
14221421
cookieLookup.getObject() = django::http::response::HttpResponse::instance() and
1423-
assign.getATarget() = subscript and
1424-
cookieLookup.flowsTo(DataFlow::exprNode(subscript.getObject())) and
1425-
index.asExpr() = subscript.getIndex() and
1426-
value.asExpr() = assign.getValue()
1422+
exists(DataFlow::Node subscriptObj |
1423+
subscriptObj.asCfgNode() = subscript.getObject()
1424+
|
1425+
cookieLookup.flowsTo(subscriptObj)
1426+
) and
1427+
value.asCfgNode() = subscript.(DefinitionNode).getValue() and
1428+
index.asCfgNode() = subscript.getIndex()
14271429
)
14281430
}
14291431

0 commit comments

Comments
 (0)