Skip to content

Commit 8daca01

Browse files
committed
C++: Cleaner use of DataFlow::Node in exprIsSubLeftOrLess.
1 parent 4879104 commit 8daca01

File tree

1 file changed

+13
-31
lines changed

1 file changed

+13
-31
lines changed

cpp/ql/src/Security/CWE/CWE-191/UnsignedDifferenceExpressionComparedZero.ql

Lines changed: 13 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -31,57 +31,39 @@ predicate isGuarded(SubExpr sub, Expr left, Expr right) {
3131
}
3232

3333
/**
34-
* Holds if `e` is known or suspected to be less than or equal to
34+
* Holds if `n` is known or suspected to be less than or equal to
3535
* `sub.getLeftOperand()`.
3636
*/
37-
predicate exprIsSubLeftOrLess(SubExpr sub, Element e) {
38-
e = sub.getLeftOperand()
37+
predicate exprIsSubLeftOrLess(SubExpr sub, DataFlow::Node n) {
38+
n = DataFlow::exprNode(sub.getLeftOperand())
3939
or
40-
exists(Expr other |
40+
exists(DataFlow::Node other |
4141
// dataflow
4242
exprIsSubLeftOrLess(sub, other) and
4343
(
44-
DataFlow::localFlowStep(DataFlow::exprNode(e), DataFlow::exprNode(other)) or
45-
DataFlow::localFlowStep(DataFlow::exprNode(other), DataFlow::exprNode(e))
44+
DataFlow::localFlowStep(n, other) or
45+
DataFlow::localFlowStep(other, n)
4646
)
4747
)
4848
or
49-
exists(Element other |
50-
// dataflow (via parameter)
51-
exprIsSubLeftOrLess(sub, other) and
52-
(
53-
DataFlow::localFlowStep(DataFlow::parameterNode(e), DataFlow::exprNode(other)) or
54-
DataFlow::localFlowStep(DataFlow::parameterNode(other), DataFlow::exprNode(e))
55-
)
56-
)
57-
or
58-
exists(Element other |
59-
// dataflow (via uninitialized)
60-
exprIsSubLeftOrLess(sub, other) and
61-
(
62-
DataFlow::localFlowStep(DataFlow::uninitializedNode(e), DataFlow::exprNode(other)) or
63-
DataFlow::localFlowStep(DataFlow::uninitializedNode(other), DataFlow::exprNode(e))
64-
)
65-
)
66-
or
67-
exists(Expr other |
49+
exists(DataFlow::Node other |
6850
// guard constraining `sub`
6951
exprIsSubLeftOrLess(sub, other) and
70-
isGuarded(sub, other, e) // other >= e
52+
isGuarded(sub, other.asExpr(), n.asExpr()) // other >= e
7153
)
7254
or
73-
exists(Expr other, float p, float q |
55+
exists(DataFlow::Node other, float p, float q |
7456
// linear access of `other`
7557
exprIsSubLeftOrLess(sub, other) and
76-
linearAccess(e, other, p, q) and // e = p * other + q
58+
linearAccess(n.asExpr(), other.asExpr(), p, q) and // e = p * other + q
7759
p <= 1 and
7860
q <= 0
7961
)
8062
or
81-
exists(Expr other, float p, float q |
63+
exists(DataFlow::Node other, float p, float q |
8264
// linear access of `e`
8365
exprIsSubLeftOrLess(sub, other) and
84-
linearAccess(other, e, p, q) and // other = p * e + q
66+
linearAccess(other.asExpr(), n.asExpr(), p, q) and // other = p * e + q
8567
p >= 1 and
8668
q >= 0
8769
)
@@ -95,5 +77,5 @@ where
9577
ro.getGreaterOperand() = sub and
9678
sub.getFullyConverted().getUnspecifiedType().(IntegralType).isUnsigned() and
9779
exprMightOverflowNegatively(sub.getFullyConverted()) and // generally catches false positives involving constants
98-
not exprIsSubLeftOrLess(sub, sub.getRightOperand()) // generally catches false positives where there's a relation between the left and right operands
80+
not exprIsSubLeftOrLess(sub, DataFlow::exprNode(sub.getRightOperand())) // generally catches false positives where there's a relation between the left and right operands
9981
select ro, "Unsigned subtraction can never be negative."

0 commit comments

Comments
 (0)