Skip to content

Commit 19e2af8

Browse files
alexrfordalexrford
authored
Merge pull request #13556 from am0o0/amammad-ruby-bombs
Ruby: Decompression Bombs
2 parents 92699d1 + dcadda2 commit 19e2af8

File tree

9 files changed

+530
-0
lines changed

9 files changed

+530
-0
lines changed

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

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,31 @@
1+
<!DOCTYPE qhelp PUBLIC
2+
"-//Semmle//qhelp//EN"
3+
"qhelp.dtd">
4+
<qhelp>
5+
<overview>
6+
<p>Extracting Compressed files with any compression algorithm like gzip can cause to denial of service attacks.</p>
7+
<p>Attackers can compress a huge file which created by repeated similiar byte and convert it to a small compressed file.</p>
8+
9+
</overview>
10+
<recommendation>
11+
12+
<p>When you want to decompress a user-provided compressed file you must be careful about the decompression ratio or read these files within a loop byte by byte to be able to manage the decompressed size in each cycle of the loop.</p>
13+
<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a></p>
14+
</recommendation>
15+
<example>
16+
<p>Rubyzip: According to <a href="https://github.com/rubyzip/rubyzip/#reading-a-zip-file">official</a> Documentation</p>
17+
<sample src="example_good.rb" />
18+
<sample src="example_bad.rb" />
19+
</example>
20+
<references>
21+
22+
<li>
23+
<a href="https://www.cvedetails.com/cve/CVE-2022-3759/">CVE-2023-22898</a>
24+
<a href="https://gitlab.com/gitlab-org/gitlab/-/issues/379633">Gitlab issue</a>
25+
</li>
26+
<li>
27+
<a href="https://www.bamsoftware.com/hacks/zipbomb/">A great research to gain more impact by this kind of attack</a>
28+
</li>
29+
30+
</references>
31+
</qhelp>

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

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,69 @@
1+
/**
2+
* @name User-controlled file decompression
3+
* @description User-controlled data that flows into decompression library APIs without checking the compression rate is dangerous
4+
* @kind path-problem
5+
* @problem.severity error
6+
* @security-severity 7.8
7+
* @precision high
8+
* @id rb/user-controlled-data-decompression
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-409
12+
*/
13+
14+
import ruby
15+
private import codeql.ruby.Concepts
16+
private import codeql.ruby.DataFlow
17+
private import codeql.ruby.TaintTracking
18+
import DecompressionBombs
19+
20+
/**
21+
* A call to `IO.copy_stream`
22+
*/
23+
class IoCopyStream extends DataFlow::CallNode {
24+
IoCopyStream() { this = API::getTopLevelMember("IO").getAMethodCall("copy_stream") }
25+
26+
DataFlow::Node getAPathArgument() { result = this.getArgument(0) }
27+
}
28+
29+
module BombsConfig implements DataFlow::ConfigSig {
30+
predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
31+
32+
predicate isSink(DataFlow::Node sink) { sink instanceof DecompressionBomb::Sink }
33+
34+
predicate isAdditionalFlowStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
35+
any(DecompressionBomb::AdditionalTaintStep ats).isAdditionalTaintStep(nodeFrom, nodeTo)
36+
or
37+
exists(API::Node n | n = API::getTopLevelMember("File").getMethod("open") |
38+
nodeFrom = n.getParameter(0).asSink() and
39+
nodeTo = n.getReturn().asSource()
40+
)
41+
or
42+
nodeFrom = nodeTo.(File::FileOpen).getAPathArgument()
43+
or
44+
exists(API::Node n | n = API::getTopLevelMember("StringIO").getMethod("new") |
45+
nodeFrom = n.getParameter(0).asSink() and
46+
nodeTo = n.getReturn().asSource()
47+
)
48+
or
49+
nodeFrom = nodeTo.(IoCopyStream).getAPathArgument()
50+
or
51+
// following can be a global additional step
52+
exists(DataFlow::CallNode cn |
53+
cn.getMethodName() = "open" and
54+
cn.getReceiver().getExprNode().getExpr() instanceof Ast::SelfVariableAccess
55+
|
56+
nodeFrom = cn.getArgument(0) and
57+
nodeTo = cn
58+
)
59+
}
60+
}
61+
62+
module Bombs = TaintTracking::Global<BombsConfig>;
63+
64+
import Bombs::PathGraph
65+
66+
from Bombs::PathNode source, Bombs::PathNode sink
67+
where Bombs::flowPath(source, sink)
68+
select sink.getNode(), source, sink, "This file Decompression depends on a $@.", source.getNode(),
69+
"potentially untrusted source"

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

Lines changed: 215 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,215 @@
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+
* A abstract class responsible for extending new decompression sinks
10+
*
11+
* can be a path, stream of compressed data,
12+
* or a call to function that use pipe
13+
*/
14+
abstract class Sink extends DataFlow::Node { }
15+
16+
/**
17+
* The additional taint steps that need for creating taint tracking or dataflow.
18+
*/
19+
abstract class AdditionalTaintStep extends string {
20+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
21+
22+
/**
23+
* Holds if there is a additional taint step between pred and succ.
24+
*/
25+
abstract predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ);
26+
}
27+
}
28+
29+
module Zlib {
30+
/**
31+
* Gets a node of `Zlib::GzipReader` member
32+
*
33+
* Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
34+
*/
35+
private API::Node gzipReaderInstance() {
36+
result = API::getTopLevelMember("Zlib").getMember("GzipReader")
37+
}
38+
39+
/**
40+
* A return values of following methods
41+
* `Zlib::GzipReader.open`
42+
* `Zlib::GzipReader.zcat`
43+
* `Zlib::GzipReader.new`
44+
*/
45+
class DecompressionBombSink extends DecompressionBomb::Sink {
46+
DecompressionBombSink() {
47+
this = gzipReaderInstance().getMethod(["open", "new", "zcat"]).getReturn().asSource()
48+
}
49+
}
50+
51+
/**
52+
* The additional taint steps that need for creating taint tracking or dataflow for `Zlib`.
53+
*/
54+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
55+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
56+
57+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
58+
exists(API::Node openOrNewOrZcat |
59+
openOrNewOrZcat = gzipReaderInstance().getMethod(["open", "new", "zcat"])
60+
|
61+
nodeFrom = openOrNewOrZcat.getParameter(0).asSink() and
62+
nodeTo = openOrNewOrZcat.getReturn().asSource()
63+
)
64+
}
65+
}
66+
}
67+
68+
module ZipInputStream {
69+
/**
70+
* Gets a node of `Zip::InputStream` member
71+
*
72+
* Note that if you use the lower level Zip::InputStream interface, rubyZip does not check the entry sizes.
73+
*/
74+
private API::Node zipInputStream() {
75+
result = API::getTopLevelMember("Zip").getMember("InputStream")
76+
}
77+
78+
/**
79+
* The methods
80+
* `Zip::InputStream.read`
81+
* `Zip::InputStream.extract`
82+
*
83+
* as source of decompression bombs, they need an additional taint step for a dataflow or taint tracking query
84+
*/
85+
class DecompressionBombSink extends DecompressionBomb::Sink {
86+
DecompressionBombSink() {
87+
this = zipInputStream().getMethod(["open", "new"]).getReturn().asSource()
88+
}
89+
}
90+
91+
/**
92+
* The additional taint steps that need for creating taint tracking or dataflow for `Zip`.
93+
*/
94+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
95+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
96+
97+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
98+
exists(API::Node openOrNew | openOrNew = zipInputStream().getMethod(["open", "new"]) |
99+
nodeFrom = openOrNew.getParameter(0).asSink() and
100+
nodeTo = openOrNew.getReturn().asSource()
101+
)
102+
}
103+
}
104+
}
105+
106+
module ZipFile {
107+
/**
108+
* Gets a node of `Zip::File` member
109+
*/
110+
API::Node zipFile() { result = API::getTopLevelMember("Zip").getMember("File") }
111+
112+
/**
113+
* Gets nodes that have a `base` at the beginning
114+
*/
115+
API::Node rubyZipNode(API::Node base) {
116+
result = base
117+
or
118+
result = rubyZipNode(base).getMethod(_)
119+
or
120+
result = rubyZipNode(base).getReturn()
121+
or
122+
result = rubyZipNode(base).getParameter(_)
123+
or
124+
result = rubyZipNode(base).getAnElement()
125+
or
126+
result = rubyZipNode(base).getBlock()
127+
}
128+
129+
/**
130+
* The return values of following methods
131+
* `ZipIO.read`
132+
* `ZipEntry.extract`
133+
* A sanitizer exists inside the nodes which have `entry.size > someOBJ`
134+
*/
135+
class DecompressionBombSink extends DecompressionBomb::Sink {
136+
DecompressionBombSink() {
137+
exists(API::Node rubyZip | rubyZip = rubyZipNode(zipFile()) |
138+
this = rubyZip.getMethod(["extract", "read"]).getReturn().asSource() and
139+
not exists(
140+
rubyZip.getMethod("size").getReturn().getMethod([">", " <", "<=", " >="]).getParameter(0)
141+
)
142+
)
143+
}
144+
}
145+
146+
/**
147+
* The additional taint steps that need for creating taint tracking or dataflow for `Zip::File`.
148+
*/
149+
class AdditionalTaintStep extends DecompressionBomb::AdditionalTaintStep {
150+
AdditionalTaintStep() { this = "AdditionalTaintStep" }
151+
152+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
153+
exists(API::Node zipFile | zipFile = zipFile().getMethod("open") |
154+
nodeFrom = zipFile.getParameter(0).asSink() and
155+
nodeTo = rubyZipNode(zipFile).getMethod(["extract", "read"]).getReturn().asSource()
156+
)
157+
}
158+
}
159+
160+
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+
// * The additional taint steps that need for creating taint tracking or dataflow for `Zip::File`.
168+
// */
169+
// class AdditionalTaintStep1 extends DecompressionBomb::AdditionalTaintStep {
170+
// AdditionalTaintStep1() { this = "AdditionalTaintStep" }
171+
// override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
172+
// exists(API::Node zipFile | zipFile = zipFile().getMethod("open") |
173+
// nodeFrom = zipFile.getParameter(0).asSink() and
174+
// nodeTo = zipFile.asSource()
175+
// )
176+
// }
177+
// }
178+
// API::Node rubyZipNode2() {
179+
// result = zipFile().getMethod("open")
180+
// or
181+
// result = rubyZipNode2().getMethod(_)
182+
// or
183+
// result = rubyZipNode2().getReturn()
184+
// or
185+
// result = rubyZipNode2().getParameter(_)
186+
// or
187+
// result = rubyZipNode2().getAnElement()
188+
// or
189+
// result = rubyZipNode2().getBlock()
190+
// }
191+
// /**
192+
// * The additional taint steps that need for creating taint tracking or dataflow for `Zip::File`.
193+
// */
194+
// class AdditionalTaintStep2 extends DecompressionBomb::AdditionalTaintStep {
195+
// AdditionalTaintStep2() { this = "AdditionalTaintStep" }
196+
// override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
197+
// exists(API::Node zipFileOpen | zipFileOpen = rubyZipNode2() |
198+
// nodeFrom = zipFileOpen.getReturn().asSource() and
199+
// nodeTo = zipFileOpen.getMethod(["extract", "read"]).getReturn().asSource()
200+
// )
201+
// }
202+
// }
203+
// /**
204+
// * The additional taint steps that need for creating taint tracking or dataflow for `Zip::File`.
205+
// */
206+
// class AdditionalTaintStep3 extends DecompressionBomb::AdditionalTaintStep {
207+
// AdditionalTaintStep3() { this = "AdditionalTaintStep" }
208+
// override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
209+
// exists(API::Node zipFileOpen | zipFileOpen = rubyZipNode2() |
210+
// nodeFrom = zipFileOpen.asCall() and
211+
// nodeTo = zipFileOpen.getMethod(["extract", "read"]).getReturn().asSource()
212+
// )
213+
// }
214+
// }
215+
}

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
# "Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes"
2+
zip_stream = Zip::InputStream.new(File.open('file.zip'))
3+
while entry = zip_stream.get_next_entry
4+
# All required operations on `entry` go here.
5+
end

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
MAX_FILE_SIZE = 10 * 1024**2 # 10MiB
2+
MAX_FILES = 100
3+
Zip::File.open('foo.zip') do |zip_file|
4+
num_files = 0
5+
zip_file.each do |entry|
6+
num_files += 1 if entry.file?
7+
raise 'Too many extracted files' if num_files > MAX_FILES
8+
raise 'File too large when extracted' if entry.size > MAX_FILE_SIZE
9+
entry.extract
10+
end
11+
end

0 commit comments

Comments
 (0)