Skip to content

Commit 75e01d3

Browse files
committed
Thanks to @owen-mc that provided a good solution of that I couldn't solve that myself
1 parent b8c8006 commit 75e01d3

File tree

2 files changed

+27
-17
lines changed

2 files changed

+27
-17
lines changed

go/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -48,17 +48,41 @@ module DecompressionBombsConfig implements DataFlow::StateConfigSig {
4848
}
4949

5050
predicate isBarrier(DataFlow::Node node) {
51-
// here I want to the CopyN return value be compared with < or > but I can't reach the tainted result
51+
// `io.CopyN` should not be a sink if its return value flows to a
52+
// comparison (<, >, <=, >=).
5253
exists(Function f, DataFlow::CallNode cn |
5354
f.hasQualifiedName("io", "CopyN") and cn = f.getACall()
5455
|
5556
node = cn.getArgument(1) and
56-
TaintTracking::localTaint(cn.getResult(0),
57-
any(DataFlow::RelationalComparisonNode rcn).getAnOperand())
57+
localStep*(cn.getResult(0), any(DataFlow::RelationalComparisonNode rcn).getAnOperand())
5858
)
5959
}
6060
}
6161

62+
/**
63+
* Holds if the value of `pred` can flow into `succ` in one step through an
64+
* arithmetic operation (other than remainder).
65+
*
66+
* Note: this predicate is copied from AllocationSizeOverflow. When this query
67+
* is promoted it should be put in a shared location.
68+
*/
69+
predicate additionalStep(DataFlow::Node pred, DataFlow::Node succ) {
70+
succ.asExpr().(ArithmeticExpr).getAnOperand() = pred.asExpr() and
71+
not succ.asExpr() instanceof RemExpr
72+
}
73+
74+
/**
75+
* Holds if the value of `pred` can flow into `succ` in one step, either by a standard taint step
76+
* or by an additional step.
77+
*
78+
* Note: this predicate is copied from AllocationSizeOverflow. When this query
79+
* is promoted it should be put in a shared location.
80+
*/
81+
predicate localStep(DataFlow::Node pred, DataFlow::Node succ) {
82+
TaintTracking::localTaintStep(pred, succ) or
83+
additionalStep(pred, succ)
84+
}
85+
6286
module DecompressionBombsFlow = TaintTracking::GlobalWithState<DecompressionBombsConfig>;
6387

6488
import DecompressionBombsFlow::PathGraph

go/ql/test/experimental/CWE-522-DecompressionBombs/DecompressionBombs.expected

Lines changed: 0 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -3,13 +3,7 @@ edges
33
| test.go:58:15:58:26 | selection of Body | test.go:136:19:136:22 | definition of file |
44
| test.go:59:16:59:27 | selection of Body | test.go:147:20:147:23 | definition of file |
55
| test.go:60:16:60:46 | call to FormValue | test.go:106:20:106:27 | definition of filename |
6-
| test.go:61:20:61:48 | call to PostFormValue | test.go:77:24:77:31 | definition of filename |
76
| test.go:63:13:63:24 | selection of Body | test.go:97:17:97:19 | definition of src |
8-
| test.go:77:24:77:31 | definition of filename | test.go:78:25:78:32 | filename |
9-
| test.go:78:2:78:33 | ... := ...[0] | test.go:81:12:81:12 | f |
10-
| test.go:78:25:78:32 | filename | test.go:78:2:78:33 | ... := ...[0] |
11-
| test.go:81:3:81:19 | ... := ...[0] | test.go:83:37:83:38 | rc |
12-
| test.go:81:12:81:12 | f | test.go:81:3:81:19 | ... := ...[0] |
137
| test.go:97:17:97:19 | definition of src | test.go:98:29:98:31 | src |
148
| test.go:98:2:98:32 | ... := ...[0] | test.go:102:11:102:26 | type conversion |
159
| test.go:98:29:98:31 | src | test.go:98:2:98:32 | ... := ...[0] |
@@ -164,14 +158,7 @@ nodes
164158
| test.go:58:15:58:26 | selection of Body | semmle.label | selection of Body |
165159
| test.go:59:16:59:27 | selection of Body | semmle.label | selection of Body |
166160
| test.go:60:16:60:46 | call to FormValue | semmle.label | call to FormValue |
167-
| test.go:61:20:61:48 | call to PostFormValue | semmle.label | call to PostFormValue |
168161
| test.go:63:13:63:24 | selection of Body | semmle.label | selection of Body |
169-
| test.go:77:24:77:31 | definition of filename | semmle.label | definition of filename |
170-
| test.go:78:2:78:33 | ... := ...[0] | semmle.label | ... := ...[0] |
171-
| test.go:78:25:78:32 | filename | semmle.label | filename |
172-
| test.go:81:3:81:19 | ... := ...[0] | semmle.label | ... := ...[0] |
173-
| test.go:81:12:81:12 | f | semmle.label | f |
174-
| test.go:83:37:83:38 | rc | semmle.label | rc |
175162
| test.go:97:17:97:19 | definition of src | semmle.label | definition of src |
176163
| test.go:98:2:98:32 | ... := ...[0] | semmle.label | ... := ...[0] |
177164
| test.go:98:29:98:31 | src | semmle.label | src |
@@ -311,7 +298,6 @@ nodes
311298
| test.go:297:25:297:31 | tarRead | semmle.label | tarRead |
312299
subpaths
313300
#select
314-
| test.go:83:37:83:38 | rc | test.go:61:20:61:48 | call to PostFormValue | test.go:83:37:83:38 | rc | This decompression is $@. | test.go:61:20:61:48 | call to PostFormValue | decompressing compressed data without managing output size |
315301
| test.go:103:23:103:28 | newSrc | test.go:63:13:63:24 | selection of Body | test.go:103:23:103:28 | newSrc | This decompression is $@. | test.go:63:13:63:24 | selection of Body | decompressing compressed data without managing output size |
316302
| test.go:112:37:112:38 | rc | test.go:60:16:60:46 | call to FormValue | test.go:112:37:112:38 | rc | This decompression is $@. | test.go:60:16:60:46 | call to FormValue | decompressing compressed data without managing output size |
317303
| test.go:125:37:125:38 | rc | test.go:60:16:60:46 | call to FormValue | test.go:125:37:125:38 | rc | This decompression is $@. | test.go:60:16:60:46 | call to FormValue | decompressing compressed data without managing output size |

0 commit comments

Comments
 (0)