Skip to content

Commit a0e839d

Browse files
committed
C++: Block duplicate taint results from 'gets' and other functions.
1 parent 06e649f commit a0e839d

5 files changed

Lines changed: 11 additions & 10 deletions

File tree

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -76,6 +76,8 @@ private class DefaultTaintTrackingCfg extends DataFlow::Configuration {
7676
}
7777

7878
override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) }
79+
80+
override predicate isBarrierIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
7981
}
8082

8183
private class ToGlobalVarTaintTrackingCfg extends DataFlow::Configuration {
@@ -96,6 +98,8 @@ private class ToGlobalVarTaintTrackingCfg extends DataFlow::Configuration {
9698
}
9799

98100
override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) }
101+
102+
override predicate isBarrierIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
99103
}
100104

101105
private class FromGlobalVarTaintTrackingCfg extends DataFlow2::Configuration {
@@ -119,6 +123,8 @@ private class FromGlobalVarTaintTrackingCfg extends DataFlow2::Configuration {
119123
}
120124

121125
override predicate isBarrier(DataFlow::Node node) { nodeIsBarrier(node) }
126+
127+
override predicate isBarrierIn(DataFlow::Node node) { nodeIsBarrierIn(node) }
122128
}
123129

124130
private predicate readsVariable(LoadInstruction load, Variable var) {
@@ -153,6 +159,11 @@ private predicate nodeIsBarrier(DataFlow::Node node) {
153159
)
154160
}
155161

162+
private predicate nodeIsBarrierIn(DataFlow::Node node) {
163+
// don't use dataflow into taint sources, as this leads to duplicate results.
164+
isUserInput(node.asExpr(), _)
165+
}
166+
156167
private predicate instructionTaintStep(Instruction i1, Instruction i2) {
157168
// Expressions computed from tainted data are also tainted
158169
exists(CallInstruction call, int argIndex | call = i2 |

cpp/ql/test/library-tests/dataflow/security-taint/tainted_diff.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,4 @@
1010
| test.cpp:68:28:68:33 | call to getenv | test.cpp:71:12:71:15 | copy | AST only |
1111
| test.cpp:87:12:87:15 | call to gets | test.cpp:87:2:87:8 | pointer | AST only |
1212
| test.cpp:87:17:87:22 | buffer | test.cpp:84:7:84:12 | buffer | AST only |
13-
| test.cpp:87:17:87:22 | buffer | test.cpp:85:8:85:14 | pointer | IR only |
14-
| test.cpp:87:17:87:22 | buffer | test.cpp:87:12:87:15 | call to gets | IR only |
1513
| test.cpp:87:17:87:22 | buffer | test.cpp:87:17:87:22 | array to pointer conversion | IR only |

cpp/ql/test/library-tests/dataflow/security-taint/tainted_ir.expected

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,5 @@
4343
| test.cpp:87:12:87:15 | call to gets | test.cpp:85:8:85:14 | pointer | |
4444
| test.cpp:87:12:87:15 | call to gets | test.cpp:87:12:87:15 | call to gets | |
4545
| test.cpp:87:17:87:22 | buffer | test.cpp:80:18:80:18 | s | |
46-
| test.cpp:87:17:87:22 | buffer | test.cpp:85:8:85:14 | pointer | |
47-
| test.cpp:87:17:87:22 | buffer | test.cpp:87:12:87:15 | call to gets | |
4846
| test.cpp:87:17:87:22 | buffer | test.cpp:87:17:87:22 | array to pointer conversion | |
4947
| test.cpp:87:17:87:22 | buffer | test.cpp:87:17:87:22 | buffer | |
Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,5 @@
11
| tests.c:28:3:28:9 | call to sprintf | This 'call to sprintf' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
22
| tests.c:29:3:29:9 | call to sprintf | This 'call to sprintf' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
3-
| tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
4-
| tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
53
| tests.c:31:15:31:23 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:31:15:31:23 | buffer100 | buffer100 |
6-
| tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:28:22:28:25 | argv | argv |
7-
| tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:29:28:29:31 | argv | argv |
8-
| tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:31:15:31:23 | buffer100 | buffer100 |
94
| tests.c:33:21:33:29 | buffer100 | This 'scanf string argument' with input from $@ may overflow the destination. | tests.c:33:21:33:29 | buffer100 | buffer100 |
105
| tests.c:34:25:34:33 | buffer100 | This 'sscanf string argument' with input from $@ may overflow the destination. | tests.c:34:10:34:13 | argv | argv |

cpp/ql/test/query-tests/Security/CWE/CWE-190/semmle/uncontrolled/ArithmeticUncontrolled.expected

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,6 @@
11
| test.c:21:17:21:17 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:18:13:18:16 | call to rand | Uncontrolled value |
22
| test.c:35:5:35:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:34:13:34:18 | call to rand | Uncontrolled value |
33
| test.c:40:5:40:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:39:13:39:21 | ... % ... | Uncontrolled value |
4-
| test.c:40:5:40:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:39:13:39:22 | call to rand | Uncontrolled value |
54
| test.c:45:5:45:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:44:13:44:16 | call to rand | Uncontrolled value |
65
| test.c:56:5:56:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:54:13:54:16 | call to rand | Uncontrolled value |
76
| test.c:67:5:67:5 | r | $@ flows to here and is used in arithmetic, potentially causing an overflow. | test.c:66:13:66:16 | call to rand | Uncontrolled value |

0 commit comments

Comments
 (0)