Ruby: Decompression Bombs #13556

am0o0 · 2023-06-24T15:23:10Z

This is part of All for one, one for all query submission, I'm going to submit an issue in github/securitylab for this pull request too.
I wrote two versions which V2 is using recursion by use of API graphs to find zip file sinks. I added V1 because I think it maybe have some advantage in your opinion but I prefer V2. the predicates in V2 can used in ZipSlip query too I can work on this too if you agree.( I couldn't write the recursion predicate without the initial help of @asgerf in #13431)
I added the only sanitizer that I think is available which is checking the size of each zip entry.
Also I added some instance of some really popular additional taint steps which I think it is good to be added as global steps.

smowton · 2023-06-24T17:52:23Z

Anyone not watching the repo in general, note this is part of a family of submissions:

#13553
#13554
#13555
#13556
#13557
#13558

am0o0 · 2023-06-25T10:33:01Z

Anyone not watching the repo in general, note this is part of a family of submissions:

#13553 #13554 #13555 #13556 #13557 #13558

#13560 is added too

github-actions · 2023-06-25T18:51:18Z

QHelp previews:

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

errors/warnings:

A fatal error occurred: Failed to read path ./ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql
(eventual cause: NoSuchFileException "./ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql")

github-advanced-security

CodeQL found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.

am0o0 · 2023-06-26T11:17:55Z

Hi, I've completed the work on this query.

calumgrant · 2023-07-11T12:18:00Z

Hey @amammad, would you be able to submit these to the Security Lab bug bounty program using this template as this will help us prioritise this work, and it would also be really helpful to test the CVEs found as well.

Thank you for your submission!

am0o0 · 2023-07-11T13:13:49Z

@calumgrant I'll submit related issues in GH security lab in this week.

github-actions · 2023-07-17T08:26:25Z

QHelp previews:

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

errors/warnings:

/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:15:4: element "p" not allowed here; expected the element end-tag or element "example", "fragment", "howToAddress", "hr", "include", "overview", "recommendation", "references", "section" or "semmleNotes"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:16:10: element "example" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:21:13: element "references" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:32:3: The element type "p" must be terminated by the matching end-tag "</p>".
A fatal error occurred: 1 qhelp files could not be processed.

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

…into amammad-ruby-bombs

am0o0 · 2023-10-17T08:48:43Z

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

+  API::Node rubyZipNode(API::Node base) {
+    result = base
+    or
+    result = rubyZipNode(base).getMethod(_)
+    or
+    result = rubyZipNode(base).getReturn()
+    or
+    result = rubyZipNode(base).getParameter(_)
+    or
+    result = rubyZipNode(base).getAnElement()
+    or
+    result = rubyZipNode(base).getBlock()
+  }


Hi, I can't use a predicate with a better performance like the following here:

API::Node rubyZipNode() { result = zipFile() or result = rubyZipNode().getMethod(_) or result = rubyZipNode().getReturn() or result = rubyZipNode().getParameter(_) or result = rubyZipNode().getAnElement() or result = rubyZipNode().getBlock() }

because lines 164 and 165 won't work for this then.

github-actions · 2023-10-27T15:50:39Z

QHelp previews:

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

errors/warnings:

/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:15:4: element "p" not allowed here; expected the element end-tag or element "example", "fragment", "howToAddress", "hr", "include", "overview", "recommendation", "references", "section" or "semmleNotes"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:16:10: element "example" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:21:13: element "references" not allowed here; expected the element end-tag, text or element "a", "b", "code", "em", "i", "img", "strong", "sub", "sup" or "tt"
/home/runner/work/codeql/codeql/ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.qhelp:32:3: The element type "p" must be terminated by the matching end-tag "</p>".
A fatal error occurred: 1 qhelp files could not be processed.

alexrford

Hi, I've run an initial review of this. Some of the feedback here may overlap with comments on the versions of this query for other languages.

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

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

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

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

alexrford · 2023-11-06T12:23:04Z

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

+
+    override DataFlow::Node sink() {
+      result = this.getMethod(["extract", "read"]).getReturn().asSource() and
+      not exists(this.getMethod("size").getReturn().getMethod(">").getParameter(0))


We should probably be using DataFlow::OperationNode and/or Ast::RelationalOperation to detect other inequality tests (e.g. >, <, <=, >=) here.

I'm also not sure that restricting this to just cases where entry.size is the greater operand is desirable. e.g. the following both function as sanitizers:

if entry.size > limit raise "exceeds limit" else entry.extract end

if entry.size < limit entry.extract else raise "exceeds limit" end

… taint steps that can improve performance

am0o0 · 2024-03-10T15:09:24Z

Hi @alexrford
Is it possible for you to review this PR before 25/3?
I have two weeks of forced vacation from 25/03 so I can focus on finishing this and other codeql PRs.

alexrford

Sorry for the delay, approved.

github-actions · 2024-05-16T12:54:51Z

QHelp previews:

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

User-controlled file decompression

Extracting Compressed files with any compression algorithm like gzip can cause to denial of service attacks.

Attackers can compress a huge file which created by repeated similiar byte and convert it to a small compressed file.

Recommendation

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.

Please read official RubyZip Documentation here

Example

Rubyzip: According to official Documentation

MAX_FILE_SIZE = 10 * 1024**2 # 10MiB
MAX_FILES = 100
Zip::File.open('foo.zip') do |zip_file|
  num_files = 0
  zip_file.each do |entry|
    num_files += 1 if entry.file?
    raise 'Too many extracted files' if num_files > MAX_FILES
    raise 'File too large when extracted' if entry.size > MAX_FILE_SIZE
    entry.extract
  end
end

# "Note that if you use the lower level Zip::InputStream interface, rubyzip does not check the entry sizes"
zip_stream = Zip::InputStream.new(File.open('file.zip'))
while entry = zip_stream.get_next_entry
  # All required operations on `entry` go here.
end

References

CVE-2023-22898 Gitlab issue
A great research to gain more impact by this kind of attack
Common Weakness Enumeration: CWE-409.

alexrford

Just a couple of suggestions to make CI pass.

alexrford · 2024-05-16T13:03:51Z

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

+</recommendation>
+<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a></p>


Suggested change

</recommendation>

<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a></p>

<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a></p>

</recommendation>

This should fix the qhelp check error.

alexrford · 2024-05-16T13:06:06Z

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

+ * @problem.severity error
+ * @security-severity 7.8
+ * @precision high
+ * @id rb/user-controlled-file-decompression


Suggested change

* @id rb/user-controlled-file-decompression

* @id rb/user-controlled-data-decompression

For consistency with JS and to address the duplicate query ID check.

alexrford · 2024-05-16T13:09:08Z

ruby/ql/test/query-tests/experimental/CWE-522-DecompressionBombs/DecompressionBombs.expected

@@ -0,0 +1,114 @@
+edges


This .expected file looks like it needs to be regenerated to account for a change in the edges relation format.

am0o0 · 2024-05-16T13:20:24Z

Sorry for the delay, approved.

Thank you a lot for approving this PR and developing and improving the CodeQL every day :)

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

+
+import ruby
+private import codeql.ruby.Concepts
+private import codeql.ruby.DataFlow


V1 Bombs

796075f

am0o0 requested a review from a team as a code owner June 24, 2023 15:23

github-actions bot added documentation Ruby labels Jun 24, 2023

This was referenced Jun 24, 2023

Go: Decompression Bombs #13553

Merged

JS: Decompression Bombs #13554

Merged

Java: Decompression Bombs #13555

Merged

This was referenced Jun 24, 2023

Python: Decompression Bombs #13557

Merged

C#: Decompression Bombs #13558

Closed

am0o0 mentioned this pull request Jun 25, 2023

C++: Decompression Bombs #13560

Merged

github-advanced-security bot found potential problems Jun 25, 2023

View reviewed changes

am0o0 added 2 commits June 26, 2023 17:14

fix formatting error/warnings

e2fe0e1

added more additional steps

9e33b47

calumgrant requested review from alexrford and asgerf June 26, 2023 08:39

RasmusWL added the external-contribution label Jun 26, 2023

am0o0 added 2 commits June 26, 2023 20:55

make one ql file

9540c58

update CVE instance in qhelp

37af588

github-advanced-security bot found potential problems Jul 17, 2023

View reviewed changes

ruby/ql/src/experimental/CWE-522-DecompressionBombs/DecompressionBombs.ql Fixed Show resolved Hide resolved

am0o0 mentioned this pull request Jul 31, 2023

[Ruby]: DOS through Decompression github/securitylab#776

Closed

2 tasks

ghsecuritylab marked this pull request as draft July 31, 2023 16:03

am0o0 added 2 commits September 6, 2023 20:17

Merge branch 'github:main' into amammad-ruby-bombs

4191b07

Merge branch 'github:main' into amammad-ruby-bombs

905fa10

am0o0 and others added 5 commits September 8, 2023 05:51

stash

d44c9d3

Merge remote-tracking branch 'origin/main' into amammad-ruby-bombs

9001771

fix a bug,modularize

609bb76

Merge branch 'amammad-ruby-bombs' of https://github.com/amammad/codeql …

8768eb6

…into amammad-ruby-bombs

fix a comment

2e4e5ef

am0o0 commented Oct 17, 2023

View reviewed changes

ghsecuritylab marked this pull request as ready for review October 18, 2023 14:33

alexrford requested changes Nov 6, 2023

View reviewed changes

apply code review suggestions, fix qldoc, add experimental additional…

2097a00

… taint steps that can improve performance

sidshank assigned alexrford Feb 1, 2024

alexrford self-requested a review May 16, 2024 12:52

alexrford approved these changes May 16, 2024

View reviewed changes

Merge branch 'main' into amammad-ruby-bombs

78dc650

alexrford requested changes May 16, 2024

View reviewed changes

am0o0 added 2 commits May 16, 2024 15:12

fix qhelp, fix duplicate query id

f06c3fd

update expected file

dcadda2

github-advanced-security bot found potential problems May 16, 2024

View reviewed changes

alexrford approved these changes May 16, 2024

View reviewed changes

alexrford merged commit 19e2af8 into github:main May 16, 2024
21 checks passed

am0o0 deleted the amammad-ruby-bombs branch September 14, 2024 11:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ruby: Decompression Bombs #13556

Ruby: Decompression Bombs #13556

am0o0 commented Jun 24, 2023 •

edited

Loading

smowton commented Jun 24, 2023

am0o0 commented Jun 25, 2023

github-actions bot commented Jun 25, 2023

errors/warnings:

github-advanced-security bot left a comment

am0o0 commented Jun 26, 2023

calumgrant commented Jul 11, 2023

am0o0 commented Jul 11, 2023

github-actions bot commented Jul 17, 2023

errors/warnings:

am0o0 Oct 17, 2023

github-actions bot commented Oct 27, 2023

errors/warnings:

alexrford left a comment

alexrford Nov 6, 2023

am0o0 commented Mar 10, 2024

alexrford left a comment

github-actions bot commented May 16, 2024 •

edited

Loading

User-controlled file decompression

Recommendation

Example

References

alexrford left a comment

alexrford May 16, 2024

alexrford May 16, 2024

alexrford May 16, 2024

am0o0 commented May 16, 2024

Check warning

		</recommendation>
		<p>Please read official RubyZip Documentation <a href="https://github.com/rubyzip/rubyzip/#size-validation">here</a></p>

	* @id rb/user-controlled-file-decompression
	* @id rb/user-controlled-data-decompression

Ruby: Decompression Bombs #13556

Ruby: Decompression Bombs #13556

Conversation

am0o0 commented Jun 24, 2023 • edited Loading

smowton commented Jun 24, 2023

am0o0 commented Jun 25, 2023

github-actions bot commented Jun 25, 2023

errors/warnings:

github-advanced-security bot left a comment

Choose a reason for hiding this comment

am0o0 commented Jun 26, 2023

calumgrant commented Jul 11, 2023

am0o0 commented Jul 11, 2023

github-actions bot commented Jul 17, 2023

errors/warnings:

am0o0 Oct 17, 2023

Choose a reason for hiding this comment

github-actions bot commented Oct 27, 2023

errors/warnings:

alexrford left a comment

Choose a reason for hiding this comment

alexrford Nov 6, 2023

Choose a reason for hiding this comment

am0o0 commented Mar 10, 2024

alexrford left a comment

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024 • edited Loading

User-controlled file decompression

Recommendation

Example

References

alexrford left a comment

Choose a reason for hiding this comment

alexrford May 16, 2024

Choose a reason for hiding this comment

alexrford May 16, 2024

Choose a reason for hiding this comment

alexrford May 16, 2024

Choose a reason for hiding this comment

am0o0 commented May 16, 2024

Check warning

am0o0 commented Jun 24, 2023 •

edited

Loading

github-actions bot commented May 16, 2024 •

edited

Loading