Skip to content

Commit 543121c

Browse files
kevinbackhousekevinbackhouse
committed
Demo of MIN_INT negate query.
1 parent 2500cfb commit 543121c

11 files changed

Lines changed: 540 additions & 0 deletions

‎CodeQL_Queries/cpp/MinIntNegate/.gitignore‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
MinIntNegateDB
2+
test.o

‎CodeQL_Queries/cpp/MinIntNegate/00_MinIntNegate.ql‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @name 00_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
12+
// Find this pattern:
13+
//
14+
// ```
15+
// if (x < 0) {
16+
// x = -x;
17+
// }
18+
// ```
19+
//
20+
// If the value of `x` is `0x80000000` then this will not make the value of `x` positive.
21+
from GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Variable v, Expr use
22+
where
23+
guard.(LTExpr).getLeftOperand() = v.getAnAccess() and
24+
guard.(LTExpr).getRightOperand().getValue().toInt() = 0 and
25+
guard.controls(block, true) and
26+
block.contains(unaryMinus) and
27+
unaryMinus.getOperand() = v.getAnAccess()
28+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive", v,
29+
v.getName()

‎CodeQL_Queries/cpp/MinIntNegate/01_MinIntNegate.ql‎

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
/**
2+
* @name 01_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
12+
// The previous query had an incorrect result at test.cpp, line 20:
13+
//
14+
// if (s->myfield < 0) {
15+
// s->myfield = -t->myfield;
16+
// }
17+
//
18+
// The problem is that the query used `Variable`, which includes fields.
19+
// So here we restrict the query to use `LocalScopeVariable` instead.
20+
from
21+
GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, LocalScopeVariable v, Expr use
22+
where
23+
guard.(LTExpr).getLeftOperand() = v.getAnAccess() and
24+
guard.(LTExpr).getRightOperand().getValue().toInt() = 0 and
25+
guard.controls(block, true) and
26+
block.contains(unaryMinus) and
27+
unaryMinus.getOperand() = v.getAnAccess()
28+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive", v,
29+
v.getName()

‎CodeQL_Queries/cpp/MinIntNegate/02_MinIntNegate.ql‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
/**
2+
* @name 02_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
12+
13+
// The previous query, 01_MinIntNegate, eliminated a bad result
14+
// from 00_MinIntNegate, but it also lost a good result.
15+
// The missing result is test.cpp, line 14:
16+
//
17+
// if (s->myfield < 0) {
18+
// s->myfield = -s->myfield;
19+
// }
20+
//
21+
// The problem is that `s->myfield` is not a `LocalScopeVariable`.
22+
// The solution is to use the GlobalValueNumbering library, which
23+
// is a more general way to find expressions that compute the same
24+
// value.
25+
from GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Expr use1, Expr use2
26+
where
27+
guard.(LTExpr).getLeftOperand() = use1 and
28+
guard.(LTExpr).getRightOperand().getValue().toInt() = 0 and
29+
guard.controls(block, true) and
30+
block.contains(unaryMinus) and
31+
unaryMinus.getOperand() = use2 and
32+
globalValueNumber(use1) = globalValueNumber(use2)
33+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive",
34+
use2, use2.toString()

‎CodeQL_Queries/cpp/MinIntNegate/03_MinIntNegate.ql‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
/**
2+
* @name 03_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
12+
13+
// The previous query only worked for `x < 0` and not for the
14+
// equivalent `0 > x`. It's easier to handle both if we refactor
15+
// the logic into a separate predicate.
16+
17+
/** Holds if `cond` is a comparison of the form `lhs < rhs`. */
18+
predicate lessThan(Expr cond, Expr lhs, Expr rhs) {
19+
cond.(LTExpr).getLeftOperand() = lhs and
20+
cond.(LTExpr).getRightOperand() = rhs
21+
or
22+
cond.(GTExpr).getLeftOperand() = rhs and
23+
cond.(GTExpr).getRightOperand() = lhs
24+
}
25+
26+
from
27+
GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Expr use1, Expr use2, Expr zero
28+
where
29+
lessThan(guard, use1, zero) and
30+
zero.getValue().toInt() = 0 and
31+
guard.controls(block, true) and
32+
block.contains(unaryMinus) and
33+
unaryMinus.getOperand() = use2 and
34+
globalValueNumber(use1) = globalValueNumber(use2)
35+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive",
36+
use2, use2.toString()

‎CodeQL_Queries/cpp/MinIntNegate/04_MinIntNegate.ql‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/**
2+
* @name 04_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
12+
13+
// Let's also add support for <= and >=.
14+
15+
/**
16+
* Holds if `cond` is a comparison of the form `lhs < rhs`.
17+
* `isStrict` is true for < and >, and false for <= and >=.
18+
*/
19+
predicate lessThan(Expr cond, Expr lhs, Expr rhs, boolean isStrict) {
20+
cond.(LTExpr).getLeftOperand() = lhs and
21+
cond.(LTExpr).getRightOperand() = rhs and
22+
isStrict = true
23+
or
24+
cond.(GTExpr).getLeftOperand() = rhs and
25+
cond.(GTExpr).getRightOperand() = lhs and
26+
isStrict = true
27+
or
28+
cond.(LEExpr).getLeftOperand() = lhs and
29+
cond.(LEExpr).getRightOperand() = rhs and
30+
isStrict = false
31+
or
32+
cond.(GEExpr).getLeftOperand() = rhs and
33+
cond.(GEExpr).getRightOperand() = lhs and
34+
isStrict = false
35+
}
36+
37+
from
38+
GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Expr use1, Expr use2, Expr zero
39+
where
40+
lessThan(guard, use1, zero, _) and
41+
zero.getValue().toInt() = 0 and
42+
guard.controls(block, true) and
43+
block.contains(unaryMinus) and
44+
unaryMinus.getOperand() = use2 and
45+
globalValueNumber(use1) = globalValueNumber(use2)
46+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive",
47+
use2, use2.toString()

‎CodeQL_Queries/cpp/MinIntNegate/05_MinIntNegate.ql‎

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
/**
2+
* @name 05_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
12+
13+
// The previous query added support for <= and >=, but failed to
14+
// find any new results. That's because the comparison is 0 <= x,
15+
// so the operands are the wrong way around. We can solve this by
16+
// adding a recursive predicate which swaps them.
17+
18+
/**
19+
* Holds if `cond` is a comparison of the form `lhs < rhs`.
20+
* `isStrict` is true for < and >, and false for <= and >=.
21+
*/
22+
predicate lessThan(Expr cond, Expr lhs, Expr rhs, boolean isStrict) {
23+
cond.(LTExpr).getLeftOperand() = lhs and
24+
cond.(LTExpr).getRightOperand() = rhs and
25+
isStrict = true
26+
or
27+
cond.(GTExpr).getLeftOperand() = rhs and
28+
cond.(GTExpr).getRightOperand() = lhs and
29+
isStrict = true
30+
or
31+
cond.(LEExpr).getLeftOperand() = lhs and
32+
cond.(LEExpr).getRightOperand() = rhs and
33+
isStrict = false
34+
or
35+
cond.(GEExpr).getLeftOperand() = rhs and
36+
cond.(GEExpr).getRightOperand() = lhs and
37+
isStrict = false
38+
}
39+
40+
/**
41+
* Holds if `cond` is a comparison of the form `lhs < rhs`.
42+
* `isStrict` is true for < and >, and false for <= and >=.
43+
* `branch` is true if the comparison is true and false if it is not.
44+
*/
45+
predicate lessThanWithNegate(Expr cond, Expr lhs, Expr rhs, boolean isStrict, boolean branch) {
46+
branch = true and lessThan(cond, lhs, rhs, isStrict)
47+
or
48+
// (x < y) == !(y <= x)
49+
lessThanWithNegate(cond, rhs, lhs, isStrict.booleanNot(), branch.booleanNot())
50+
}
51+
52+
from
53+
GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Expr use1, Expr use2,
54+
Expr zero, boolean branch
55+
where
56+
lessThanWithNegate(guard, use1, zero, _, branch) and
57+
zero.getValue().toInt() = 0 and
58+
guard.controls(block, branch) and
59+
block.contains(unaryMinus) and
60+
unaryMinus.getOperand() = use2 and
61+
globalValueNumber(use1) = globalValueNumber(use2)
62+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive",
63+
use2, use2.toString()

‎CodeQL_Queries/cpp/MinIntNegate/06_MinIntNegate.ql‎

Lines changed: 75 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,75 @@
1+
/**
2+
* @name 06_MinIntNegate
3+
* @description Negating MIN_INT is an integer overflow
4+
* @kind problem
5+
* @id cpp/min-int-negate
6+
* @problem.severity warning
7+
*/
8+
9+
import cpp
10+
import semmle.code.cpp.controlflow.Guards
11+
import semmle.code.cpp.valuenumbering.GlobalValueNumbering
12+
import semmle.code.cpp.dataflow.DataFlow
13+
14+
// Let's add local dataflow, so that we can also handle cases like this:
15+
//
16+
// ```
17+
// bool b = x < 0;
18+
// if (b) {
19+
// x = -x;
20+
// }
21+
// ```
22+
23+
/**
24+
* Holds if `cond` is a comparison of the form `lhs < rhs`.
25+
* `isStrict` is true for < and >, and false for <= and >=.
26+
*/
27+
predicate lessThan(Expr cond, Expr lhs, Expr rhs, boolean isStrict) {
28+
cond.(LTExpr).getLeftOperand() = lhs and
29+
cond.(LTExpr).getRightOperand() = rhs and
30+
isStrict = true
31+
or
32+
cond.(GTExpr).getLeftOperand() = rhs and
33+
cond.(GTExpr).getRightOperand() = lhs and
34+
isStrict = true
35+
or
36+
cond.(LEExpr).getLeftOperand() = lhs and
37+
cond.(LEExpr).getRightOperand() = rhs and
38+
isStrict = false
39+
or
40+
cond.(GEExpr).getLeftOperand() = rhs and
41+
cond.(GEExpr).getRightOperand() = lhs and
42+
isStrict = false
43+
}
44+
45+
/**
46+
* Holds if `cond` is a comparison of the form `lhs < rhs`.
47+
* `isStrict` is true for < and >, and false for <= and >=.
48+
* `branch` is true if the comparison is true and false if it is not.
49+
*/
50+
predicate lessThanWithNegate(Expr cond, Expr lhs, Expr rhs, boolean isStrict, boolean branch) {
51+
branch = true and lessThan(cond, lhs, rhs, isStrict)
52+
or
53+
// (x < y) == !(y <= x)
54+
lessThanWithNegate(cond, rhs, lhs, isStrict.booleanNot(), branch.booleanNot())
55+
or
56+
// bool b = x < 0;
57+
// if (b) { ... }
58+
exists(Expr prev |
59+
DataFlow::localExprFlow(prev, cond) and
60+
lessThanWithNegate(prev, lhs, rhs, branch, isStrict)
61+
)
62+
}
63+
64+
from
65+
GuardCondition guard, BasicBlock block, UnaryMinusExpr unaryMinus, Expr use1, Expr use2,
66+
Expr zero, boolean branch
67+
where
68+
lessThanWithNegate(guard, use1, zero, _, branch) and
69+
zero.getValue().toInt() = 0 and
70+
guard.controls(block, branch) and
71+
block.contains(unaryMinus) and
72+
unaryMinus.getOperand() = use2 and
73+
globalValueNumber(use1) = globalValueNumber(use2)
74+
select unaryMinus, "If the value of $@ is MinInt then this assignment will not make it positive",
75+
use2, use2.toString()

0 commit comments

Comments
 (0)