Skip to content

Commit 768cab3

Browse files
authored
Python: Address review comments
- changes `getReceiver` to `getObject` - fixes `calls` to avoid unwanted cross-talk - adds some more documentation to highlight the above issue
1 parent 359bc5e commit 768cab3

File tree

7 files changed

+63
-13
lines changed

7 files changed

+63
-13
lines changed

python/ql/src/Security/CWE-327/Ssl.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,7 @@ API::Node sslContextInstance() {
4242
class WrapSocketCall extends ConnectionCreation, DataFlow::MethodCallNode {
4343
WrapSocketCall() { this = sslContextInstance().getMember("wrap_socket").getACall() }
4444

45-
override DataFlow::Node getContext() { result = this.getReceiver() }
45+
override DataFlow::Node getContext() { result = this.getObject() }
4646
}
4747

4848
class OptionsAugOr extends ProtocolRestriction, DataFlow::CfgNode {

python/ql/src/experimental/semmle/python/frameworks/Stdlib.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -70,7 +70,7 @@ private module Re {
7070
CompiledRegex() {
7171
exists(DataFlow::MethodCallNode patternCall |
7272
patternCall = API::moduleImport("re").getMember("compile").getACall() and
73-
patternCall.flowsTo(this.getReceiver()) and
73+
patternCall.flowsTo(this.getObject()) and
7474
this.getMethodName() instanceof RegexExecutionMethods and
7575
regexNode = patternCall.getArg(0)
7676
)

python/ql/src/semmle/python/dataflow/new/internal/DataFlowPublic.qll

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -184,23 +184,38 @@ class CallCfgNode extends CfgNode, LocalSourceNode {
184184
* A data-flow node corresponding to a method call, that is `foo.bar(...)`.
185185
*
186186
* Also covers the case where the method lookup is done separately from the call itself, as in
187-
* `temp = foo.bar; temp(...)`.
187+
* `temp = foo.bar; temp(...)`. Note that this is only tracked through local scope.
188188
*/
189189
class MethodCallNode extends CallCfgNode {
190190
AttrRead method_lookup;
191191

192192
MethodCallNode() { method_lookup = this.getFunction().getALocalSource() }
193193

194-
/** Gets the name of the method being invoked (the `bar` in `foo.bar(...)`, if it can be determined. */
194+
/**
195+
* Gets the name of the method being invoked (the `bar` in `foo.bar(...)`) if it can be determined.
196+
*
197+
* Note that this method may have multiple results if a single call node represents calls to
198+
* multiple different objects and methods. If you want to link up objects and method names
199+
* accurately, use the `calls` method instead.
200+
*/
195201
string getMethodName() { result = method_lookup.getAttributeName() }
196202

197-
/** Gets the data-flow node corresponding to the receiver of this call. That is, the `foo` in `foo.bar(...)`. */
198-
Node getReceiver() { result = method_lookup.getObject() }
199-
200-
/** Holds if this data-flow node calls method `methodName` on receiver node `receiver`. */
201-
predicate calls(Node receiver, string methodName) {
202-
receiver = this.getReceiver() and
203-
methodName = this.getMethodName()
203+
/**
204+
* Gets the data-flow node corresponding to the object receiving this call. That is, the `foo` in
205+
* `foo.bar(...)`.
206+
*
207+
* Note that this method may have multiple results if a single call node represents calls to
208+
* multiple different objects and methods. If you want to link up objects and method names
209+
* accurately, use the `calls` method instead.
210+
*/
211+
Node getObject() { result = method_lookup.getObject() }
212+
213+
/** Holds if this data-flow node calls method `methodName` on the object node `object`. */
214+
predicate calls(Node object, string methodName) {
215+
// As `getObject` and `getMethodName` may both have multiple results, we must look up the object
216+
// and method name directly on `method_lookup`.
217+
object = method_lookup.getObject() and
218+
methodName = method_lookup.getAttributeName()
204219
}
205220
}
206221

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

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -267,8 +267,11 @@ private module CryptographyModel {
267267
string algorithmName;
268268

269269
CryptographyGenericCipherOperation() {
270-
this.getMethodName() in ["update", "update_into"] and
271-
this.getReceiver() in [cipherEncryptor(algorithmName), cipherDecryptor(algorithmName)]
270+
exists(DataFlow::Node object, string method |
271+
object in [cipherEncryptor(algorithmName), cipherDecryptor(algorithmName)] and
272+
method in ["update", "update_into"] and
273+
this.calls(object, method)
274+
)
272275
}
273276

274277
override Cryptography::CryptographicAlgorithm getAlgorithm() {
Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
conjunctive_lookup
2+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj1 | bar |
3+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj1 | foo |
4+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj2 | bar |
5+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj2 | foo |
6+
calls_lookup
7+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj1 | foo |
8+
| test.py:6:1:6:6 | ControlFlowNode for meth() | meth() | obj2 | bar |
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
if cond:
2+
meth = obj1.foo
3+
else:
4+
meth = obj2.bar
5+
6+
meth()
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
import python
2+
import semmle.python.dataflow.new.DataFlow
3+
import experimental.dataflow.TestUtil.PrintNode
4+
5+
query predicate conjunctive_lookup(
6+
DataFlow::MethodCallNode methCall, string call, string object, string methodName
7+
) {
8+
call = prettyNode(methCall) and
9+
object = prettyNode(methCall.getObject()) and
10+
methodName = methCall.getMethodName()
11+
}
12+
13+
query predicate calls_lookup(
14+
DataFlow::MethodCallNode methCall, string call, string object, string methodName
15+
) {
16+
call = prettyNode(methCall) and
17+
exists(DataFlow::Node o | methCall.calls(o, methodName) and object = prettyNode(o))
18+
}

0 commit comments

Comments
 (0)