Skip to content

Commit 609bb76

Browse files
am0o0am0o0
committed
fix a bug,modularize
1 parent 9001771 commit 609bb76

File tree

5 files changed

+360
-284
lines changed

5 files changed

+360
-284
lines changed

‎ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql

Lines changed: 18 additions & 141 deletions
Original file line numberDiff line numberDiff line change
@@ -11,124 +11,10 @@
1111
* external/cwe/cwe-409
1212
*/
1313

14-
import codeql.ruby.AST
15-
import codeql.ruby.frameworks.Files
16-
import codeql.ruby.ApiGraphs
17-
import codeql.ruby.DataFlow
18-
import codeql.ruby.dataflow.RemoteFlowSources
19-
import codeql.ruby.TaintTracking
20-
import DataFlow::PathGraph
21-
22-
module DecompressionBombs {
23-
abstract class DecompressionBombSink extends DataFlow::Node { }
24-
25-
module Zlib {
26-
/**
27-
* `Zlib::GzipReader`
28-
* > Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
29-
*
30-
* according to above warning from Doc we don't need to go forward after open()
31-
* or new() methods, we just need the argument node of them
32-
*/
33-
private API::Node gzipReaderInstance() {
34-
result = API::getTopLevelMember("Zlib").getMember("GzipReader")
35-
}
36-
37-
/**
38-
* A return values of following methods
39-
* `Zlib::GzipReader.open`
40-
* `Zlib::GzipReader.zcat`
41-
* `Zlib::GzipReader.new`
42-
*/
43-
class ZipSink extends DecompressionBombSink {
44-
ZipSink() {
45-
this = gzipReaderInstance().getMethod(["open", "new", "zcat"]).getReturn().asSource()
46-
}
47-
}
48-
49-
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
50-
exists(API::Node zipnode | zipnode = gzipReaderInstance().getMethod(["open", "new", "zcat"]) |
51-
nodeFrom = zipnode.getParameter(0).asSink() and
52-
nodeTo = zipnode.getReturn().asSource()
53-
)
54-
}
55-
}
56-
57-
module ZipInputStream {
58-
/**
59-
* `Zip::InputStream`
60-
* > Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
61-
*
62-
* according to above warning from Doc we don't need to go forward after open()
63-
* or new() methods, we just need the argument node of them
64-
*/
65-
private API::Node zipInputStream() {
66-
result = API::getTopLevelMember("Zip").getMember("InputStream")
67-
}
68-
69-
/**
70-
* A return values of following methods
71-
* `ZipIO.read`
72-
* `ZipEntry.extract`
73-
*/
74-
class ZipSink extends DecompressionBombSink {
75-
ZipSink() {
76-
this = zipInputStream().getMethod(["open", "new"]).getReturn().asSource() and
77-
not this.getLocation().getFile().getBaseName().matches("%spec%")
78-
}
79-
}
80-
81-
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
82-
exists(API::Node zipnode | zipnode = zipInputStream().getMethod(["open", "new"]) |
83-
nodeFrom = zipnode.getParameter(0).asSink() and
84-
nodeTo = zipnode.getReturn().asSource()
85-
)
86-
}
87-
}
88-
89-
module ZipFile {
90-
API::Node rubyZipNode() {
91-
result = zipFile()
92-
or
93-
result = rubyZipNode().getMethod(_)
94-
or
95-
result = rubyZipNode().getReturn()
96-
or
97-
result = rubyZipNode().getParameter(_)
98-
or
99-
result = rubyZipNode().getAnElement()
100-
or
101-
result = rubyZipNode().getBlock()
102-
}
103-
104-
/**
105-
* A return values of following methods
106-
* `ZipIO.read`
107-
* `ZipEntry.extract`
108-
* sanitize the nodes which have `entry.size > someOBJ`
109-
*/
110-
class ZipSink extends DecompressionBombSink {
111-
ZipSink() {
112-
exists(API::Node zipnodes | zipnodes = rubyZipNode() |
113-
this = zipnodes.getMethod(["extract", "read"]).getReturn().asSource() and
114-
not exists(zipnodes.getMethod("size").getReturn().getMethod(">").getParameter(0))
115-
)
116-
}
117-
}
118-
119-
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
120-
exists(API::Node zipnodes | zipnodes = rubyZipNode() |
121-
nodeTo = zipnodes.getMethod(["extract", "read"]).getReturn().asSource() and
122-
nodeFrom = zipnodes.getMethod(["new", "open"]).getParameter(0).asSink()
123-
)
124-
}
125-
126-
/**
127-
* `Zip::File`
128-
*/
129-
API::Node zipFile() { result = API::getTopLevelCall("Zip").getMember("File") }
130-
}
131-
}
14+
private import codeql.ruby.Concepts
15+
private import codeql.ruby.DataFlow
16+
private import codeql.ruby.TaintTracking
17+
import DecompressionBombs
13218

13319
/**
13420
* A call to `IO.copy_stream`
@@ -139,30 +25,17 @@ class IoCopyStream extends DataFlow::CallNode {
13925
DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
14026
}
14127

142-
class Bombs extends TaintTracking::Configuration {
143-
Bombs() { this = "Decompression Bombs" }
144-
145-
override predicate isSanitizer(DataFlow::Node node) {
146-
not node.getLocation().hasLocationInfo("%spec%", _, _, _, _)
28+
module BombsConfig implements DataFlow::ConfigSig {
29+
predicate isBarrier(DataFlow::Node node) {
30+
node.getLocation().hasLocationInfo("%spec%", _, _, _, _)
14731
}
14832

149-
override predicate isSource(DataFlow::Node source) {
150-
source instanceof RemoteFlowSource
151-
// or
152-
// source instanceof DataFlow::LocalSourceNode
153-
// source = API::getTopLevelCall("Zip").getMember("InputStream").getASuccessor*()
154-
}
33+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
15534

156-
override predicate isSink(DataFlow::Node sink) {
157-
sink instanceof DecompressionBombs::DecompressionBombSink
158-
}
35+
predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionBomb::Sink }
15936

160-
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
161-
DecompressionBombs::ZipFile::isAdditionalTaintStep(nodeFrom, nodeTo)
162-
or
163-
DecompressionBombs::ZipInputStream::isAdditionalTaintStep(nodeFrom, nodeTo)
164-
or
165-
DecompressionBombs::Zlib::isAdditionalTaintStep(nodeFrom, nodeTo)
37+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
38+
any(DecompressionBomb::AdditionalTaintStep ats).isAdditionalTaintStep(nodeFrom, nodeTo)
16639
or
16740
exists(API::Node n | n = API::root().getMember("File").getMethod("open") |
16841
nodeFrom = n.getParameter(0).asSink() and
@@ -194,7 +67,11 @@ class Bombs extends TaintTracking::Configuration {
19467
}
19568
}
19669

197-
from Bombs cfg, DataFlow::PathNode source, DataFlow::PathNode sink
198-
where cfg.hasFlowPath(source, sink)
199-
select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(),
70+
module Bombs = TaintTracking::Global<BombsConfig>;
71+
72+
import Bombs::PathGraph
73+
74+
from Bombs::PathNode source, Bombs::PathNode sink
75+
where Bombs::flowPath(source, sink)
76+
select sink.getNode(), source, sink, "This file Decompression depends on a $@.", source.getNode(),
20077
"potentially untrusted source"

‎ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qll

Lines changed: 167 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,167 @@
1+
import codeql.ruby.AST
2+
import codeql.ruby.frameworks.Files
3+
import codeql.ruby.ApiGraphs
4+
import codeql.ruby.DataFlow
5+
import codeql.ruby.dataflow.RemoteFlowSources
6+
7+
module DecompressionBomb {
8+
/**
9+
* The Sinks of uncontrolled data decompression
10+
*/
11+
class Sink extends DataFlow::Node {
12+
Sink() { this = any(Range r).sink() }
13+
}
14+
15+
/**
16+
* The additional taint steps that need for creating taint tracking or dataflow.
17+
*/
18+
abstract class AdditionalTaintStep extends string {
19+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
20+
21+
/**
22+
* Holds if there is a additional taint step between pred and succ.
23+
*/
24+
abstract predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ);
25+
}
26+
27+
/**
28+
* A abstract class responsible for extending new decompression sinks
29+
*/
30+
abstract class Range extends API::Node {
31+
/**
32+
* Gets the sink of responsible for decompression node
33+
*
34+
* it can be a path, stream of compressed data,
35+
* or a call to function that use pipe
36+
*/
37+
abstract DataFlow::Node sink();
38+
}
39+
}
40+
41+
module Zlib {
42+
/**
43+
* Gets a node of `Zlib::GzipReader` member
44+
*
45+
* Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
46+
*/
47+
private API::Node gzipReaderInstance() {
48+
result = API::getTopLevelMember("Zlib").getMember("GzipReader")
49+
}
50+
51+
/**
52+
* A return values of following methods
53+
* `Zlib::GzipReader.open`
54+
* `Zlib::GzipReader.zcat`
55+
* `Zlib::GzipReader.new`
56+
*/
57+
class DecompressionBombSink extends DecompressionBomb::Range {
58+
DecompressionBombSink() { this = gzipReaderInstance().getMethod(["open", "new", "zcat"]) }
59+
60+
override DataFlow::Node sink() { result = this.getReturn().asSource() }
61+
}
62+
63+
/**
64+
* The additional taint steps that need for creating taint tracking or dataflow for `Zlib`.
65+
*/
66+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
67+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
68+
69+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
70+
exists(API::Node openOrNewOrZcat |
71+
openOrNewOrZcat = gzipReaderInstance().getMethod(["open", "new", "zcat"])
72+
|
73+
nodeFrom = openOrNewOrZcat.getParameter(0).asSink() and
74+
nodeTo = openOrNewOrZcat.getReturn().asSource()
75+
)
76+
}
77+
}
78+
}
79+
80+
module ZipInputStream {
81+
/**
82+
* Gets a node of `Zip::InputStream` member
83+
*
84+
* Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
85+
*/
86+
private API::Node zipInputStream() {
87+
result = API::getTopLevelMember("Zip").getMember("InputStream")
88+
}
89+
90+
/**
91+
* The return values of following methods
92+
* `ZipIO.read`
93+
* `ZipEntry.extract`
94+
*/
95+
class DecompressionBombSink extends DecompressionBomb::Range {
96+
DecompressionBombSink() { this = zipInputStream().getMethod(["open", "new"]) }
97+
98+
override DataFlow::Node sink() { result = this.getReturn().asSource() }
99+
}
100+
101+
/**
102+
* The additional taint steps that need for creating taint tracking or dataflow for `Zip`.
103+
*/
104+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
105+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
106+
107+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
108+
exists(API::Node openOrNew | openOrNew = zipInputStream().getMethod(["open", "new"]) |
109+
nodeFrom = openOrNew.getParameter(0).asSink() and
110+
nodeTo = openOrNew.getReturn().asSource()
111+
)
112+
}
113+
}
114+
}
115+
116+
module ZipFile {
117+
/**
118+
* Gets a node of `Zip::File` member
119+
*/
120+
API::Node zipFile() { result = API::getTopLevelMember("Zip").getMember("File") }
121+
122+
/**
123+
* Gets nodes that have a `base` at the beginning
124+
*/
125+
API::Node rubyZipNode(API::Node base) {
126+
result = base
127+
or
128+
result = rubyZipNode(base).getMethod(_)
129+
or
130+
result = rubyZipNode(base).getReturn()
131+
or
132+
result = rubyZipNode(base).getParameter(_)
133+
or
134+
result = rubyZipNode(base).getAnElement()
135+
or
136+
result = rubyZipNode(base).getBlock()
137+
}
138+
139+
/**
140+
* The return values of following methods
141+
* `ZipIO.read`
142+
* `ZipEntry.extract`
143+
* A sanitizer exists inside the nodes which have `entry.size > someOBJ`
144+
*/
145+
class DecompressionBombSink extends DecompressionBomb::Range {
146+
DecompressionBombSink() { this = rubyZipNode(zipFile()) }
147+
148+
override DataFlow::Node sink() {
149+
result = this.getMethod(["extract", "read"]).getReturn().asSource() and
150+
not exists(this.getMethod("size").getReturn().getMethod(">").getParameter(0))
151+
}
152+
}
153+
154+
/**
155+
* The additional taint steps that need for creating taint tracking or dataflow for `Zip::File`.
156+
*/
157+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
158+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
159+
160+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
161+
exists(API::Node zipFile | zipFile = zipFile().getMethod("open") |
162+
nodeFrom = zipFile.getParameter(0).asSink() and
163+
nodeTo = rubyZipNode(zipFile).getMethod(["extract", "read"]).getReturn().asSource()
164+
)
165+
}
166+
}
167+
}

0 commit comments

Comments
 (0)