Skip to content

Commit c5d2866

Browse files
MathiasVPMathiasVP
authored
Merge pull request github#14812 from MathiasVP/no-dtt-in-Integer-overflow-tainted
C++: Convert `cpp/integer-overflow-tainted` away from DefaultTaintTracking
2 parents ca33402 + da2215e commit c5d2866

File tree

4 files changed

+102
-30
lines changed

4 files changed

+102
-30
lines changed

‎cpp/ql/src/Security/CWE/CWE-190/IntegerOverflowTainted.ql

Lines changed: 77 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,11 @@
1515

1616
import cpp
1717
import semmle.code.cpp.rangeanalysis.SimpleRangeAnalysis
18-
import semmle.code.cpp.ir.dataflow.internal.DefaultTaintTrackingImpl
18+
import semmle.code.cpp.dataflow.new.DataFlow
19+
import semmle.code.cpp.security.FlowSources as FS
20+
import semmle.code.cpp.dataflow.new.TaintTracking
21+
import semmle.code.cpp.ir.IR
22+
import semmle.code.cpp.controlflow.IRGuards as IRGuards
1923

2024
/** Holds if `expr` might overflow. */
2125
predicate outOfBoundsExpr(Expr expr, string kind) {
@@ -27,13 +31,76 @@ predicate outOfBoundsExpr(Expr expr, string kind) {
2731
else none()
2832
}
2933

30-
from Expr use, Expr origin, string kind
34+
predicate isSource(FS::FlowSource source, string sourceType) { sourceType = source.getSourceType() }
35+
36+
predicate isSink(DataFlow::Node sink, string kind) {
37+
exists(Expr use |
38+
use = sink.asExpr() and
39+
not use.getUnspecifiedType() instanceof PointerType and
40+
outOfBoundsExpr(use, kind) and
41+
not inSystemMacroExpansion(use)
42+
)
43+
}
44+
45+
predicate hasUpperBoundsCheck(Variable var) {
46+
exists(RelationalOperation oper, VariableAccess access |
47+
oper.getAnOperand() = access and
48+
access.getTarget() = var and
49+
// Comparing to 0 is not an upper bound check
50+
not oper.getAnOperand().getValue() = "0"
51+
)
52+
}
53+
54+
predicate constantInstruction(Instruction instr) {
55+
instr instanceof ConstantInstruction or
56+
constantInstruction(instr.(UnaryInstruction).getUnary())
57+
}
58+
59+
predicate readsVariable(LoadInstruction load, Variable var) {
60+
load.getSourceAddress().(VariableAddressInstruction).getAstVariable() = var
61+
}
62+
63+
predicate nodeIsBarrierEqualityCandidate(DataFlow::Node node, Operand access, Variable checkedVar) {
64+
exists(Instruction instr | instr = node.asInstruction() |
65+
readsVariable(instr, checkedVar) and
66+
any(IRGuards::IRGuardCondition guard).ensuresEq(access, _, _, instr.getBlock(), true)
67+
)
68+
}
69+
70+
module Config implements DataFlow::ConfigSig {
71+
predicate isSource(DataFlow::Node source) { isSource(source, _) }
72+
73+
predicate isSink(DataFlow::Node sink) { isSink(sink, _) }
74+
75+
predicate isBarrier(DataFlow::Node node) {
76+
// Block flow if there's an upper bound check of the variable anywhere in the program
77+
exists(Variable checkedVar, Instruction instr | instr = node.asInstruction() |
78+
readsVariable(instr, checkedVar) and
79+
hasUpperBoundsCheck(checkedVar)
80+
)
81+
or
82+
// Block flow if the node is guarded by an equality check
83+
exists(Variable checkedVar, Operand access |
84+
nodeIsBarrierEqualityCandidate(node, access, checkedVar) and
85+
readsVariable(access.getDef(), checkedVar)
86+
)
87+
or
88+
// Block flow to any binary instruction whose operands are both non-constants.
89+
exists(BinaryInstruction iTo |
90+
iTo = node.asInstruction() and
91+
not constantInstruction(iTo.getLeft()) and
92+
not constantInstruction(iTo.getRight()) and
93+
// propagate taint from either the pointer or the offset, regardless of constantness
94+
not iTo instanceof PointerArithmeticInstruction
95+
)
96+
}
97+
}
98+
99+
module Flow = TaintTracking::Global<Config>;
100+
101+
from DataFlow::Node source, DataFlow::Node sink, string kind, string sourceType
31102
where
32-
not use.getUnspecifiedType() instanceof PointerType and
33-
outOfBoundsExpr(use, kind) and
34-
tainted(origin, use) and
35-
origin != use and
36-
not inSystemMacroExpansion(use) and
37-
// Avoid double-counting: don't include all the conversions of `use`.
38-
not use instanceof Conversion
39-
select use, "$@ flows an expression which might " + kind + ".", origin, "User-provided value"
103+
Flow::flow(source, sink) and
104+
isSource(source, sourceType) and
105+
isSink(sink, kind)
106+
select sink, "$@ flows an expression which might " + kind + ".", source, sourceType

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/SAMATE/IntegerOverflowTainted.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
| examples.cpp:66:9:66:14 | -- ... | $@ flows an expression which might overflow negatively. | examples.cpp:63:26:63:30 | & ... | User-provided value |
1+
| examples.cpp:66:9:66:14 | -- ... | $@ flows an expression which might overflow negatively. | examples.cpp:63:26:63:30 | fscanf output argument | value read by fscanf |

‎cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/tainted/IntegerOverflowTainted.expected

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,23 @@
1-
| test2.cpp:14:11:14:15 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
2-
| test2.cpp:15:11:15:19 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
3-
| test2.cpp:16:11:16:21 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
4-
| test2.cpp:17:11:17:22 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | & ... | User-provided value |
5-
| test2.cpp:39:9:39:18 | ... + ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
6-
| test2.cpp:40:3:40:13 | ... += ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | buffer | User-provided value |
7-
| test3.c:12:31:12:34 | * ... | $@ flows an expression which might overflow negatively. | test3.c:11:15:11:18 | argv | User-provided value |
8-
| test3.c:13:16:13:19 | * ... | $@ flows an expression which might overflow negatively. | test3.c:11:15:11:18 | argv | User-provided value |
9-
| test4.cpp:13:17:13:20 | access to array | $@ flows an expression which might overflow negatively. | test4.cpp:9:13:9:16 | argv | User-provided value |
10-
| test5.cpp:10:9:10:15 | call to strtoul | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
11-
| test5.cpp:17:6:17:27 | ... * ... | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
12-
| test5.cpp:19:6:19:13 | ... * ... | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | buf | User-provided value |
13-
| test6.cpp:11:15:11:15 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | & ... | User-provided value |
14-
| test6.cpp:16:15:16:15 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | & ... | User-provided value |
15-
| test6.cpp:30:16:30:16 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | & ... | User-provided value |
16-
| test.c:14:15:14:35 | ... * ... | $@ flows an expression which might overflow. | test.c:11:29:11:32 | argv | User-provided value |
17-
| test.c:44:7:44:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:41:17:41:20 | argv | User-provided value |
18-
| test.c:54:7:54:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:51:17:51:20 | argv | User-provided value |
1+
| test2.cpp:14:11:14:15 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf |
2+
| test2.cpp:15:11:15:19 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf |
3+
| test2.cpp:16:11:16:21 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf |
4+
| test2.cpp:17:11:17:22 | ... * ... | $@ flows an expression which might overflow. | test2.cpp:25:22:25:23 | fscanf output argument | value read by fscanf |
5+
| test2.cpp:39:9:39:18 | ... + ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | fgets output argument | string read by fgets |
6+
| test2.cpp:40:3:40:13 | ... += ... | $@ flows an expression which might overflow. | test2.cpp:36:9:36:14 | fgets output argument | string read by fgets |
7+
| test3.c:12:11:12:34 | * ... | $@ flows an expression which might overflow negatively. | test3.c:10:27:10:30 | argv indirection | a command-line argument |
8+
| test3.c:12:11:12:34 | * ... | $@ flows an expression which might overflow negatively. | test.c:10:27:10:30 | argv indirection | a command-line argument |
9+
| test3.c:13:11:13:20 | * ... | $@ flows an expression which might overflow negatively. | test3.c:10:27:10:30 | argv indirection | a command-line argument |
10+
| test3.c:13:11:13:20 | * ... | $@ flows an expression which might overflow negatively. | test.c:10:27:10:30 | argv indirection | a command-line argument |
11+
| test4.cpp:13:7:13:20 | access to array | $@ flows an expression which might overflow negatively. | test4.cpp:8:27:8:30 | argv indirection | a command-line argument |
12+
| test5.cpp:10:9:10:27 | call to strtoul | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets |
13+
| test5.cpp:17:6:17:27 | ... * ... | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets |
14+
| test5.cpp:19:6:19:13 | ... * ... | $@ flows an expression which might overflow. | test5.cpp:9:7:9:9 | gets output argument | string read by gets |
15+
| test6.cpp:11:10:11:15 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | fscanf output argument | value read by fscanf |
16+
| test6.cpp:16:10:16:15 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | fscanf output argument | value read by fscanf |
17+
| test6.cpp:30:11:30:16 | s | $@ flows an expression which might overflow. | test6.cpp:39:23:39:24 | fscanf output argument | value read by fscanf |
18+
| test.c:14:15:14:35 | ... * ... | $@ flows an expression which might overflow. | test3.c:10:27:10:30 | argv indirection | a command-line argument |
19+
| test.c:14:15:14:35 | ... * ... | $@ flows an expression which might overflow. | test.c:10:27:10:30 | argv indirection | a command-line argument |
20+
| test.c:44:7:44:12 | ... -- | $@ flows an expression which might overflow negatively. | test3.c:10:27:10:30 | argv indirection | a command-line argument |
21+
| test.c:44:7:44:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:10:27:10:30 | argv indirection | a command-line argument |
22+
| test.c:54:7:54:12 | ... -- | $@ flows an expression which might overflow negatively. | test3.c:10:27:10:30 | argv indirection | a command-line argument |
23+
| test.c:54:7:54:12 | ... -- | $@ flows an expression which might overflow negatively. | test.c:10:27:10:30 | argv indirection | a command-line argument |

‎cpp/ql/test/query-tests/Security/CWE/CWE-197/SAMATE/IntegerOverflowTainted/IntegerOverflowTainted.expected

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1 +1 @@
1-
| tests.cpp:38:31:38:34 | data | $@ flows an expression which might overflow. | tests.cpp:57:27:57:31 | & ... | User-provided value |
1+
| tests.cpp:38:25:38:34 | data | $@ flows an expression which might overflow. | tests.cpp:57:27:57:31 | fscanf output argument | value read by fscanf |

0 commit comments

Comments
 (0)