Skip to content

Commit 6bf38ef

Browse files
committed
Fix Allocation Size Overflow for use-use flow
We have an operator expression like `x * 5`. We want to follow where the value of the operator expression goes. We used to follow local flow from an operand, but now there is flow from that operand to the next use of the variable. The fix is to explicitly start local flow from the operator expression. There are also some expected edge changes due to use-use flow.
1 parent ca98c4a commit 6bf38ef

File tree

2 files changed

+22
-11
lines changed

2 files changed

+22
-11
lines changed

go/ql/lib/semmle/go/security/AllocationSizeOverflowCustomizations.qll

Lines changed: 14 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,10 @@ module AllocationSizeOverflow {
3232
/**
3333
* A data-flow node that is an operand to an operation that may overflow.
3434
*/
35-
abstract class OverflowProneOperand extends DataFlow::Node { }
35+
abstract class OverflowProneOperand extends DataFlow::Node {
36+
/** Gets the operation that may overflow that `this` is an operand of. */
37+
abstract DataFlow::Node getOverflowProneOperation();
38+
}
3639

3740
/**
3841
* A data-flow node that represents the size argument of an allocation, such as the `n` in
@@ -91,8 +94,7 @@ module AllocationSizeOverflow {
9194
AllocationSize allocsz;
9295

9396
DefaultSink() {
94-
this instanceof OverflowProneOperand and
95-
localStep*(this, allocsz) and
97+
localStep*(this.(OverflowProneOperand).getOverflowProneOperation(), allocsz) and
9698
not allocsz instanceof AllocationSizeCheckBarrier
9799
}
98100

@@ -134,15 +136,18 @@ module AllocationSizeOverflow {
134136

135137
/** An operand of an arithmetic expression that could cause overflow. */
136138
private class DefaultOverflowProneOperand extends OverflowProneOperand {
139+
OperatorExpr parent;
140+
137141
DefaultOverflowProneOperand() {
138-
exists(OperatorExpr parent | isOverflowProne(parent) |
139-
this.asExpr() = parent.getAnOperand() and
140-
// only consider outermost operands to avoid double reporting
141-
not exists(OperatorExpr grandparent | parent = grandparent.getAnOperand().stripParens() |
142-
isOverflowProne(grandparent)
143-
)
142+
isOverflowProne(parent) and
143+
this.asExpr() = parent.getAnOperand() and
144+
// only consider outermost operands to avoid double reporting
145+
not exists(OperatorExpr grandparent | parent = grandparent.getAnOperand().stripParens() |
146+
isOverflowProne(grandparent)
144147
)
145148
}
149+
150+
override DataFlow::Node getOverflowProneOperation() { result.asExpr() = parent }
146151
}
147152

148153
/**

go/ql/test/query-tests/Security/CWE-190/AllocationSizeOverflow.expected

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,13 @@ edges
66
| tst2.go:14:2:14:29 | ... := ...[0] | tst2.go:15:26:15:29 | data |
77
| tst2.go:15:26:15:29 | data | tst2.go:15:22:15:30 | call to len |
88
| tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:7:26:7:33 | jsonData |
9-
| tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:24:20:24:27 | jsonData |
10-
| tst3.go:6:2:6:31 | ... := ...[0] | tst3.go:32:20:32:27 | jsonData |
119
| tst3.go:7:26:7:33 | jsonData | tst3.go:7:22:7:34 | call to len |
10+
| tst3.go:7:26:7:33 | jsonData | tst3.go:9:32:9:39 | jsonData |
11+
| tst3.go:9:32:9:39 | jsonData | tst3.go:11:9:11:16 | jsonData |
12+
| tst3.go:11:9:11:16 | jsonData | tst3.go:16:20:16:27 | jsonData |
13+
| tst3.go:16:20:16:27 | jsonData | tst3.go:24:20:24:27 | jsonData |
1214
| tst3.go:24:20:24:27 | jsonData | tst3.go:24:16:24:28 | call to len |
15+
| tst3.go:24:20:24:27 | jsonData | tst3.go:32:20:32:27 | jsonData |
1316
| tst3.go:32:20:32:27 | jsonData | tst3.go:32:16:32:28 | call to len |
1417
| tst.go:14:2:14:30 | ... = ...[0] | tst.go:15:26:15:33 | jsonData |
1518
| tst.go:15:26:15:33 | jsonData | tst.go:15:22:15:34 | call to len |
@@ -32,6 +35,9 @@ nodes
3235
| tst3.go:6:2:6:31 | ... := ...[0] | semmle.label | ... := ...[0] |
3336
| tst3.go:7:22:7:34 | call to len | semmle.label | call to len |
3437
| tst3.go:7:26:7:33 | jsonData | semmle.label | jsonData |
38+
| tst3.go:9:32:9:39 | jsonData | semmle.label | jsonData |
39+
| tst3.go:11:9:11:16 | jsonData | semmle.label | jsonData |
40+
| tst3.go:16:20:16:27 | jsonData | semmle.label | jsonData |
3541
| tst3.go:24:16:24:28 | call to len | semmle.label | call to len |
3642
| tst3.go:24:20:24:27 | jsonData | semmle.label | jsonData |
3743
| tst3.go:32:16:32:28 | call to len | semmle.label | call to len |

0 commit comments

Comments
 (0)