Skip to content

Commit 832abc7

Browse files
authored
Merge pull request #1473 from markshannon/python-points-to-more-unknowns
Python: Fix getOperand for 'not' node and make sure it can only point-to a boolean.
2 parents 4d77902 + 5b145ed commit 832abc7

File tree

6 files changed

+15
-13
lines changed

6 files changed

+15
-13
lines changed

python/ql/src/semmle/python/Flow.qll

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -707,10 +707,14 @@ class UnaryExprNode extends ControlFlowNode {
707707
toAst(this) instanceof UnaryExpr
708708
}
709709

710-
/** flow node corresponding to the operand of a unary expression */
710+
/** Gets flow node corresponding to the operand of a unary expression.
711+
* Note that this might not be the flow node for the AST operand.
712+
* In `not (a or b)` the AST operand is `(a or b)`, but as `a or b` is
713+
* a short-circuiting operation, there will be two `not` CFG nodes, one will
714+
* have `a` or `b` as it operand, the other will have just `b`.
715+
*/
711716
ControlFlowNode getOperand() {
712-
exists(UnaryExpr u | this.getNode() = u and result.getNode() = u.getOperand()) and
713-
result.getBasicBlock().dominates(this.getBasicBlock())
717+
result = this.getAPredecessor()
714718
}
715719

716720
override UnaryExpr getNode() { result = super.getNode() }

python/ql/src/semmle/python/pointsto/PointsTo.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1238,7 +1238,7 @@ module Expressions {
12381238
or
12391239
op instanceof USub and value = ObjectInternal::fromInt(-opvalue.intValue())
12401240
or
1241-
opvalue = ObjectInternal::unknown() and value = opvalue
1241+
not op instanceof Not and opvalue = ObjectInternal::unknown() and value = opvalue
12421242
) and
12431243
origin = u
12441244
}

python/ql/test/library-tests/PointsTo/new/PointsToUnknown.expected

Lines changed: 1 addition & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,7 @@
22
| a_simple.py:24 | ControlFlowNode for x | 23 |
33
| a_simple.py:29 | ControlFlowNode for x | 27 |
44
| a_simple.py:35 | ControlFlowNode for Subscript | 35 |
5-
| a_simple.py:35 | ControlFlowNode for UnaryExpr | 35 |
65
| a_simple.py:36 | ControlFlowNode for Subscript | 36 |
7-
| a_simple.py:36 | ControlFlowNode for UnaryExpr | 36 |
86
| b_condition.py:5 | ControlFlowNode for IfExp | 5 |
97
| b_condition.py:5 | ControlFlowNode for cond | 5 |
108
| b_condition.py:5 | ControlFlowNode for unknown | 5 |
@@ -25,7 +23,6 @@
2523
| b_condition.py:17 | ControlFlowNode for cond | 17 |
2624
| b_condition.py:17 | ControlFlowNode for unknown | 17 |
2725
| b_condition.py:17 | ControlFlowNode for unknown() | 17 |
28-
| b_condition.py:19 | ControlFlowNode for UnaryExpr | 19 |
2926
| b_condition.py:19 | ControlFlowNode for x | 17 |
3027
| b_condition.py:21 | ControlFlowNode for use | 21 |
3128
| b_condition.py:21 | ControlFlowNode for use() | 21 |
@@ -48,7 +45,6 @@
4845
| b_condition.py:31 | ControlFlowNode for cond | 31 |
4946
| b_condition.py:31 | ControlFlowNode for unknown | 31 |
5047
| b_condition.py:31 | ControlFlowNode for unknown() | 31 |
51-
| b_condition.py:32 | ControlFlowNode for UnaryExpr | 32 |
5248
| b_condition.py:32 | ControlFlowNode for x | 31 |
5349
| b_condition.py:34 | ControlFlowNode for use | 34 |
5450
| b_condition.py:34 | ControlFlowNode for use() | 34 |
@@ -89,7 +85,6 @@
8985
| b_condition.py:70 | ControlFlowNode for cond | 70 |
9086
| b_condition.py:70 | ControlFlowNode for unknown | 70 |
9187
| b_condition.py:70 | ControlFlowNode for unknown() | 70 |
92-
| b_condition.py:71 | ControlFlowNode for UnaryExpr | 71 |
9388
| b_condition.py:71 | ControlFlowNode for b | 70 |
9489
| b_condition.py:73 | ControlFlowNode for b | 70 |
9590
| b_condition.py:79 | ControlFlowNode for use | 79 |
@@ -112,8 +107,8 @@
112107
| b_condition.py:97 | ControlFlowNode for x | 87 |
113108
| b_condition.py:99 | ControlFlowNode for use | 99 |
114109
| b_condition.py:99 | ControlFlowNode for use() | 99 |
110+
| b_condition.py:99 | ControlFlowNode for x | 87 |
115111
| b_condition.py:105 | ControlFlowNode for Subscript | 105 |
116-
| b_condition.py:105 | ControlFlowNode for UnaryExpr | 105 |
117112
| b_condition.py:110 | ControlFlowNode for Attribute | 110 |
118113
| b_condition.py:110 | ControlFlowNode for Attribute() | 110 |
119114
| b_condition.py:110 | ControlFlowNode for x | 109 |
@@ -178,7 +173,6 @@
178173
| c_tests.py:83 | ControlFlowNode for cond | 83 |
179174
| c_tests.py:83 | ControlFlowNode for unknown | 83 |
180175
| c_tests.py:83 | ControlFlowNode for unknown() | 83 |
181-
| c_tests.py:84 | ControlFlowNode for UnaryExpr | 84 |
182176
| c_tests.py:84 | ControlFlowNode for b | 83 |
183177
| c_tests.py:85 | ControlFlowNode for b | 83 |
184178
| c_tests.py:87 | ControlFlowNode for unknown | 87 |
@@ -193,7 +187,6 @@
193187
| c_tests.py:94 | ControlFlowNode for cond | 94 |
194188
| c_tests.py:94 | ControlFlowNode for unknown | 94 |
195189
| c_tests.py:94 | ControlFlowNode for unknown() | 94 |
196-
| c_tests.py:95 | ControlFlowNode for UnaryExpr | 95 |
197190
| c_tests.py:95 | ControlFlowNode for x | 94 |
198191
| c_tests.py:99 | ControlFlowNode for bar | 99 |
199192
| c_tests.py:99 | ControlFlowNode for bar() | 99 |
@@ -222,7 +215,6 @@
222215
| r_regressions.py:29 | ControlFlowNode for x | 27 |
223216
| r_regressions.py:31 | ControlFlowNode for y | 27 |
224217
| r_regressions.py:33 | ControlFlowNode for y | 27 |
225-
| r_regressions.py:35 | ControlFlowNode for UnaryExpr | 35 |
226218
| r_regressions.py:36 | ControlFlowNode for z | 27 |
227219
| r_regressions.py:39 | ControlFlowNode for use | 39 |
228220
| r_regressions.py:39 | ControlFlowNode for use() | 39 |

python/ql/test/library-tests/PointsTo/new/PointsToWithContext.expected

100755100644
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P
159159
| b_condition.py:87 | ControlFlowNode for split_bool1 | Function split_bool1 | builtin-class function | 87 | import |
160160
| b_condition.py:88 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 | runtime |
161161
| b_condition.py:88 | ControlFlowNode for y | NoneType None | builtin-class NoneType | 87 | runtime |
162+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | bool False | builtin-class bool | 90 | runtime |
163+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | bool True | builtin-class bool | 90 | runtime |
162164
| b_condition.py:90 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 | runtime |
163165
| b_condition.py:90 | ControlFlowNode for y | NoneType None | builtin-class NoneType | 87 | runtime |
164166
| b_condition.py:92 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 | runtime |

python/ql/test/library-tests/PointsTo/new/PointsToWithType.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -159,6 +159,8 @@ WARNING: Predicate points_to has been deprecated and may be removed in future (P
159159
| b_condition.py:87 | ControlFlowNode for split_bool1 | Function split_bool1 | builtin-class function | 87 |
160160
| b_condition.py:88 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 |
161161
| b_condition.py:88 | ControlFlowNode for y | NoneType None | builtin-class NoneType | 87 |
162+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | bool False | builtin-class bool | 90 |
163+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | bool True | builtin-class bool | 90 |
162164
| b_condition.py:90 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 |
163165
| b_condition.py:90 | ControlFlowNode for y | NoneType None | builtin-class NoneType | 87 |
164166
| b_condition.py:92 | ControlFlowNode for x | NoneType None | builtin-class NoneType | 87 |

python/ql/test/library-tests/PointsTo/new/Values.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -115,6 +115,8 @@
115115
| b_condition.py:87 | ControlFlowNode for None | import | None | builtin-class NoneType |
116116
| b_condition.py:88 | ControlFlowNode for x | runtime | None | builtin-class NoneType |
117117
| b_condition.py:88 | ControlFlowNode for y | runtime | None | builtin-class NoneType |
118+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | runtime | bool False | builtin-class bool |
119+
| b_condition.py:90 | ControlFlowNode for UnaryExpr | runtime | bool True | builtin-class bool |
118120
| b_condition.py:90 | ControlFlowNode for x | runtime | None | builtin-class NoneType |
119121
| b_condition.py:90 | ControlFlowNode for y | runtime | None | builtin-class NoneType |
120122
| b_condition.py:92 | ControlFlowNode for x | runtime | None | builtin-class NoneType |

0 commit comments

Comments
 (0)