Skip to content

Commit 796075f

Browse files
am0o0am0o0
committed
V1 Bombs
1 parent cc09715 commit 796075f

File tree

11 files changed

+1003
-0
lines changed

11 files changed

+1003
-0
lines changed

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

Lines changed: 479 additions & 0 deletions
Large diffs are not rendered by default.

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

Lines changed: 179 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,179 @@
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-file-decompression
9+
* @tags security
10+
* experimental
11+
* external/cwe/cwe-409
12+
*/
13+
14+
import codeql.ruby.AST
15+
import codeql.ruby.ApiGraphs
16+
import codeql.ruby.DataFlow
17+
import codeql.ruby.dataflow.RemoteFlowSources
18+
import codeql.ruby.TaintTracking
19+
import DataFlow::PathGraph
20+
21+
module DecompressionBombs {
22+
abstract class DecompressionBombSink extends DataFlow::Node { }
23+
24+
module Zlib {
25+
/**
26+
* `Zlib::GzipReader`
27+
* > Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
28+
*
29+
* according to above warning from Doc we don't need to go forward after open()
30+
* or new() methods, we just need the argument node of them
31+
*/
32+
private API::Node gzipReaderInstance() {
33+
result = API::getTopLevelMember("Zlib").getMember("GzipReader")
34+
}
35+
36+
/**
37+
* return values of following methods
38+
* `Zlib::GzipReader.open`
39+
* `Zlib::GzipReader.zcat`
40+
* `Zlib::GzipReader.new`
41+
*/
42+
class ZipSink extends DecompressionBombSink {
43+
ZipSink() {
44+
this = gzipReaderInstance().getMethod(["open", "new", "zcat"]).getReturn().asSource()
45+
}
46+
}
47+
48+
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
49+
exists(API::Node zipnode | zipnode = gzipReaderInstance().getMethod(["open", "new", "zcat"]) |
50+
nodeFrom = zipnode.getParameter(0).asSink() and
51+
nodeTo = zipnode.getReturn().asSource()
52+
)
53+
}
54+
}
55+
56+
module ZipInputStream {
57+
/**
58+
* `Zip::InputStream`
59+
* > Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes.
60+
*
61+
* according to above warning from Doc we don't need to go forward after open()
62+
* or new() methods, we just need the argument node of them
63+
*/
64+
private API::Node zipInputStream() {
65+
result = API::getTopLevelMember("Zip").getMember("InputStream")
66+
}
67+
68+
/**
69+
* return values of following methods
70+
* `ZipIO.read`
71+
* `ZipEntry.extract`
72+
*/
73+
class ZipSink extends DecompressionBombSink {
74+
ZipSink() { this = zipInputStream().getMethod(["open", "new"]).getReturn().asSource() }
75+
}
76+
77+
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
78+
exists(API::Node zipnode | zipnode = zipInputStream().getMethod(["open", "new"]) |
79+
nodeFrom = zipnode.getParameter(0).asSink() and
80+
nodeTo = zipnode.getReturn().asSource()
81+
)
82+
}
83+
}
84+
85+
module ZipFile {
86+
// // Because of additional step and ZipSink predicates, I couldn't use unary predicate
87+
// // I put the explanation because I think there should be a soloution to not use other rubyZipNode predicate
88+
// API::Node rubyZipNode() {
89+
// result = zipFile() or
90+
// result = rubyZipNode().getMethod(_).getReturn() or
91+
// result = rubyZipNode().getMethod(_).getBlock().getParameter(_) or
92+
// result = rubyZipNode().getMethod(_).getParameter(0) or
93+
// result = rubyZipNode().getAnElement()
94+
// }
95+
API::Node rubyZipNode(API::Node n) {
96+
result = n
97+
or
98+
result = rubyZipNode(n).getMethod(_).getReturn()
99+
or
100+
result = rubyZipNode(n).getMethod(_).getBlock().getParameter(_)
101+
or
102+
result = rubyZipNode(n).getMethod(_).getParameter(0)
103+
or
104+
result = rubyZipNode(n).getAnElement()
105+
}
106+
107+
/**
108+
* return values of following methods
109+
* `ZipIO.read`
110+
* `ZipEntry.extract`
111+
* sanitize the nodes which have `entry.size > someOBJ`
112+
*/
113+
class ZipSink extends DecompressionBombSink {
114+
ZipSink() {
115+
exists(API::Node zipnodes | zipnodes = zipFile() |
116+
this = rubyZipNode(zipnodes).getMethod(["extract", "read"]).getReturn().asSource() and
117+
not exists(
118+
rubyZipNode(zipnodes).getMethod("size").getReturn().getMethod(">").getParameter(0)
119+
)
120+
)
121+
}
122+
}
123+
124+
predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
125+
exists(API::Node zipnodes | zipnodes = zipFile() |
126+
nodeTo = [rubyZipNode(zipnodes).getMethod(["extract", "read"]).getReturn().asSource()] and
127+
nodeFrom = zipnodes.getMethod(["new", "open"]).getParameter(0).asSink()
128+
)
129+
}
130+
131+
/**
132+
* `Zip::File`
133+
*/
134+
private API::Node zipFile() { result = API::getTopLevelMember("Zip").getMember("File") }
135+
}
136+
}
137+
138+
class Bombs extends TaintTracking::Configuration {
139+
Bombs() { this = "Decompression Bombs" }
140+
141+
override predicate isSource(DataFlow::Node source) {
142+
source instanceof RemoteFlowSource or
143+
source instanceof DataFlow::LocalSourceNode
144+
}
145+
146+
override predicate isSink(DataFlow::Node sink) {
147+
sink instanceof DecompressionBombs::DecompressionBombSink
148+
}
149+
150+
override predicate isAdditionalTaintStep(DataFlow::Node nodeFrom, DataFlow::Node nodeTo) {
151+
DecompressionBombs::ZipFile::isAdditionalTaintStep(nodeFrom, nodeTo)
152+
or
153+
DecompressionBombs::ZipInputStream::isAdditionalTaintStep(nodeFrom, nodeTo)
154+
or
155+
DecompressionBombs::Zlib::isAdditionalTaintStep(nodeFrom, nodeTo)
156+
or
157+
exists(API::Node n | n = API::root().getMember("File").getMethod("open") |
158+
nodeFrom = n.getParameter(0).asSink() and
159+
nodeTo = n.getReturn().asSource()
160+
)
161+
or
162+
exists(API::Node n | n = API::root().getMember("StringIO").getMethod("new") |
163+
nodeFrom = n.getParameter(0).asSink() and
164+
nodeTo = n.getReturn().asSource()
165+
)
166+
or
167+
exists(DataFlow::CallNode cn |
168+
cn.getMethodName() = "open" and cn.getReceiver().toString() = "self"
169+
|
170+
nodeFrom = cn.getArgument(0) and
171+
nodeTo = cn
172+
)
173+
}
174+
}
175+
176+
from Bombs cfg, DataFlow::PathNode source, DataFlow::PathNode sink
177+
where cfg.hasFlowPath(source, sink)
178+
select sink.getNode(), source, sink, "This file extraction depends on a $@.", source.getNode(),
179+
"potentially untrusted source"

‎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+
14+
</recommendation>
15+
<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a>
16+
<example>
17+
<p>Rubyzip: According to <a href="https://github.com/rubyzip/rubyzip/#reading-a-zip-file">official</a> Documentation</p>
18+
<sample src="example_good.rb" />
19+
<sample src="example_bad.rb" />
20+
</example>
21+
<references>
22+
23+
<li>
24+
<a href="https://nvd.nist.gov/vuln/detail/CVE-2023-22898">CVE-2023-22898</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/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)