Skip to content

Commit 08f78a2

Browse files
committed
fix some flowstate bug which Had caused to FP
1 parent 0f540f4 commit 08f78a2

1 file changed

Lines changed: 23 additions & 27 deletions

File tree

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

Lines changed: 23 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -27,17 +27,12 @@ module DecompressionBombs implements DataFlow::StateConfigSig {
2727
// or
2828
// exists(Parameter p | p.getARead() = source | p.hasQualifiedName("io", "Reader"))
2929
) and
30-
state =
31-
[
32-
"ZstdNewReader", "XzNewReader", "GzipNewReader", "S2NewReader", "SnapyNewReader",
33-
"ZlibNewReader", "FlateNewReader", "Bzip2NewReader", "ZipOpenReader", "IOMethods",
34-
"ZipKlauspost"
35-
]
30+
state = ""
3631
or
3732
exists(DataFlow::Function f |
3833
(
3934
f.hasQualifiedName("archive/zip", ["OpenReader", "NewReader"]) and
40-
state = ""
35+
state = "ZipOpenReader"
4136
or
4237
f.hasQualifiedName("github.com/klauspost/compress/zip", ["NewReader", "OpenReader"]) and
4338
state = "ZipKlauspost"
@@ -145,24 +140,6 @@ module DecompressionBombs implements DataFlow::StateConfigSig {
145140
]
146141
}
147142

148-
predicate isBarrier(DataFlow::Node node, FlowState state) {
149-
//here I want to the CopyN return value be compared with < or > but I can't reach the tainted result
150-
// exists(DataFlow::Function f | f.hasQualifiedName("io", "CopyN") |
151-
// node = f.getACall().getArgument([0, 1]) and
152-
// TaintTracking::localExprTaint(f.getACall().getResult(_).asExpr(),
153-
// any(RelationalComparisonExpr e).getAChildExpr*())
154-
// )
155-
// or
156-
exists(DataFlow::Function f | f.hasQualifiedName("io", "LimitReader") |
157-
node = f.getACall().getArgument(0) and f.getACall().getArgument(1).isConst()
158-
) and
159-
state =
160-
[
161-
"ZstdNewReader", "XzNewReader", "GzipNewReader", "S2NewReader", "SnapyNewReader",
162-
"ZlibNewReader", "FlateNewReader", "Bzip2NewReader", "ZipOpenReader", "ZipKlauspost"
163-
]
164-
}
165-
166143
predicate isAdditionalFlowStep(DataFlow::Node fromNode, DataFlow::Node toNode) {
167144
exists(DataFlow::FieldReadNode fi |
168145
fi.getType().hasQualifiedName("github.com/klauspost/compress/zip", "Reader")
@@ -291,6 +268,25 @@ module DecompressionBombs implements DataFlow::StateConfigSig {
291268
toState = "S2NewReader"
292269
)
293270
}
271+
272+
predicate isBarrier(DataFlow::Node node, FlowState state) {
273+
// //here I want to the CopyN return value be compared with < or > but I can't reach the tainted result
274+
// // exists(DataFlow::Function f | f.hasQualifiedName("io", "CopyN") |
275+
// // node = f.getACall().getArgument([0, 1]) and
276+
// // TaintTracking::localExprTaint(f.getACall().getResult(_).asExpr(),
277+
// // any(RelationalComparisonExpr e).getAChildExpr*())
278+
// // )
279+
// // or
280+
// exists(DataFlow::Function f | f.hasQualifiedName("io", "LimitReader") |
281+
// node = f.getACall().getArgument(0) and f.getACall().getArgument(1).isConst()
282+
// ) and
283+
// state =
284+
// [
285+
// "ZstdNewReader", "XzNewReader", "GzipNewReader", "S2NewReader", "SnapyNewReader",
286+
// "ZlibNewReader", "FlateNewReader", "Bzip2NewReader", "ZipOpenReader", "ZipKlauspost"
287+
// ]
288+
none()
289+
}
294290
}
295291

296292
// // here I want to the CopyN return value be compared with < or > but I can't reach the tainted result
@@ -310,5 +306,5 @@ import DecompressionBombsFlow::PathGraph
310306

311307
from DecompressionBombsFlow::PathNode source, DecompressionBombsFlow::PathNode sink
312308
where DecompressionBombsFlow::flowPath(source, sink)
313-
select sink.getNode(), source, sink, "This file extraction $@.", source.getNode(),
314-
"decompressing data controlling output size"
309+
select sink.getNode(), source, sink, "This decompression $@.", source.getNode(),
310+
"decompressing compressed data without managing output size"

0 commit comments

Comments
 (0)