rpms/ruby
7
0
Fork 0
Code Issues Pull Requests Releases Activity
fd513df176
ruby/ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch
Jarek Prokop 46b6a33dba Fix RCE vulnerability with .rdoc_options in RDoc (CVE-2024-27281). 
The patch contains 2 commits, 1 from upstream that comes from
7957a25edf
which is a commit present in Ruby 3.0 branch. That is the closest Ruby
version to 2.5 that contains the CVE fix.

2nd fix is made as a downstream adjustment to the YAML calls used
in the fix.
While upstream relies on `YAML::safe_load_file` method, Ruby 2.5.9's
YAML module contains no such method.
To not introduce new RHEL exclusive downstream logic into the RDoc library,
the `YAML::safe_load_file` method was copied from Ruby 3.3.0's
sources: https://github.com/ruby/ruby/blob/v3_3_0/ext/psych/lib/psych.rb#L658

In psych this was first introduced in:
0210e310d0 (diff-659eac8589abc82c9a0ab3699e4e4be4774d9c09c6c9934af5d9dae0d264439cR592)
In Ruby 3.0 this capability was added with this commit:
c2a60fec2f (diff-87e1cb3017e15fc7f32c0bc0144934204bde9a56849dcd0cde864118e95ea38cR591)
Since then it had not changed up to Ruby 3.3, therefore it does not make a
difference whether the method was copied from Ruby 3.0.0 or Ruby 3.3.0.

The method is very simple, it is a `File.open` call that then provides the open
file as a readable object containing YAML. One more thing of note is
`r:bom|utf-8`, which essentially opens the YAML file in reading mode
utilizing Unicode BOM to help determine encoding, if the BOM is missing,
the utf-8 is used instead. 2.5 documentation Reference:
https://docs.ruby-lang.org/en/2.5.0/IO.html#method-c-new-label-IO+Encoding
. Additionally, in the YAML function there is a non-optional argument `filename`.
It is used to give more information in exceptions with the `YAML::safe_load`.

While the `YAML::safe_load_file` did not change from Ruby 3.0.0 up to at least
the marked 3.3.0 commit, `YAML::safe_load` went through a function argument
transformation, where, between Ruby 2.6 and the 3.0 timeframe the arguments
that can be provided to the method changed from regular positional
arguments to keyword arguments. Upstream's CVE fix, both in the test
files and in the actual source code counts with this capability. This is
especially important to the `permitted_classes:` keyword argument, that
is part of the CVE's mitigation.

To keep upstream logic even in Ruby 2.5, we have to:
1) Copy the YAML.safe_load_file logic
2) Provide the arguments to `YAML::safe_load` correctly to keep with the
   `YAML::safe_load_file` behavior.

Doing step 1 is a bit tricky. We do not want to backport that method, as
that would extend Ruby 2.5.9's capabilities beyond what was intended
upstream.

Therefore it seems better, as the `YAML.safe_load_file` method is only
called once in that entire upstream patch, to just copy-paste the logic.
This is then only internal to RDoc and achieves functional parity with
the upstream patch.

For step 2, it is necessary to keep in mind how `YAML::safe_load` looks
in Ruby 2.5 vs Ruby 3.0--Ruby 3.3
Ruby 2.5:
https://github.com/ruby/ruby/blob/v2_5_9/ext/psych/lib/psych.rb#L313
Ruby 3.0:
https://github.com/ruby/ruby/blob/v3_0_0/ext/psych/lib/psych.rb#L329
Ruby 3.3:
https://github.com/ruby/ruby/blob/v3_3_0/ext/psych/lib/psych.rb#L322

And also how the arguments will be provided to `YAML::safe_load_file`.

While Ruby 2.5 has all the arguments for `safe_load` positional,
Ruby 3.0 has a mix, where they have deprecated positional arguments in
favor of keyword arguments, but kept the positional arugments around for that
time. Judging by the prepended "legacy_" on the arguments, they were
kept around for compatibility.
Ruby 3.3 does not contain positional arguments at all and only contains
keyword arguments

Let's consider the difference between Ruby 2.5's and Ruby 3.3.0's `YAML::safe_load`.
(While we could use Ruby 3.0.0 here, it is better to use Ruby 3.3. The
newer Ruby provides us with a clearer look at the method arguments. Mainly caused
by the fact that upstream only uses keyword arguments, so for the
purposes of the upstream CVE patch that only uses keyword arguments,
they are effectively identical for us)
Then let's also consider what is required to provide to the copy-pasted
`YAML::safe_load` internal to `YAML::safe_load_file` from Ruby 3.3
to keep equivalent semantic in RDoc.

Ruby 2.5 defines the function with arguments as:
~~~
  def self.safe_load yaml, whitelist_classes = [], whitelist_symbols = [], aliases = false, filename = nil, symbolize_names: false
~~~
and Ruby 3.3 defines the function with arguments as:
~~~
  def self.safe_load yaml, permitted_classes: [], permitted_symbols: [], aliases: false, filename: nil, fallback: nil, symbolize_names: false, freeze: false, strict_integer: false
~~~

We should focus on 2 things. Defaults and naming.
Naming changed due to
682abf20f0
but despite that, the ordering remained the same for the arguments we care
about so we can compare them more directly when we look past the
difference of positional vs keyword arguments.

From the Ruby 3.3 `YAML::safe_load_file`'s definition
~~~
  def self.safe_load_file filename, **kwargs
    File.open(filename, 'r:bom|utf-8') { |f|
      self.safe_load f, filename: filename, **kwargs
    }
  end
~~~
we already can assume that we will need to provide the correct filename
to both the File.open and the `YAML::safe_load` since it is provided
here.

From the upstream RDoc CVE patch:
7957a25edf (diff-a4c824fbc4e6e86217cec1b940a70536a4134afc08cbd8083a0340407d714b3aR165)
we can see the way `YAML::safe_load_file` is used in `lib/rdoc/rdoc.rb`:
~~~
      options = YAML.safe_load_file '.rdoc_options', permitted_classes: [RDoc::Options, Symbol]
~~~
We know that the first argument for the `YAML::safe_load_file` is
`filename`, and `permitted_classes` argument is passed into the method.
As hinted earlier, keyword arguments other than filename are passed in via
`**kwargs`, so there is no special handling around them and it is like
we would be calling the underlying `YAML::safe_load` with that keyword
argument directly.

Also studying the differences in `YAML::safe_load` between Ruby 2.5 and 3.3
as noted in the Ruby documentation:
Ruby 2.5: https://docs.ruby-lang.org/en/2.5.0/Psych.html#method-c-safe_load
Ruby 3.3: https://docs.ruby-lang.org/en/3.3/Psych.html#method-c-safe_load
the `whitelist_classes` in 2.5 and `permitted_classes` in 3.3 are
equivalent and are serving the same purpose.
So, we can provide the value to permitted_classes as the 2nd
positional argument to `YAML::safe_load` without using the keyword.
This applies also to the downstream test file adjustments.

However, to achieve parity with the upstream code in `lib/rdoc/rdoc.rb` we
also have to pay attention to correctly utilizing the `filename` argument.
The upstream patch only uses the `.rdoc_options` file, therefore
we can safely hardcode that value into the calls, replacing the argument in the
mentioned `YAML::safe_load_file` downstream implementation.

However, in Ruby 2.5 we also have to pay attention to where `filename` is
provided. There are multiple keyword arguments that are not used
upstream, we are not using them as well. However since the arguments are
positional we have to provide default values to the arguments preceding
the `filename` argument in the `YAML::safe_load` call to keep the
behavior as it was implemented upstream.

Therefore our downstream `YAML::safe_load_file` replacement in the
`lib/rdoc/rdoc.rb` looks like this:
~~~
      options = File.open('.rdoc_options', 'r:bom|utf-8') do |file|
        YAML.safe_load file, [RDoc::Options, Symbol], [], false, '.rdoc_options'
      end
~~~
If we imagine this without `options = ` this is almost the same as the
Ruby 3.3 `YAML.safe_load_file`. The key difference here is hardcoded
`filename` arguments to both `File::open` and `YAML::safe_load`.
But as `filename` is the 5th argument and we only filled the first 2
arguments, we have to fill the arguments in-between with their default
values. Namely, upstream does not utilize equivalents to
`whitelist_symbols` and `aliases`. Their default are, in-order, `[]` and
`false`, so we insert them in between. Then we provide the '.rdoc_options'
as the `filename`, to make sure it will be provided in a potential exception
from YAML in the case of having failed to parse the file.

Resolves: RHEL-34117
2024-06-24 19:39:15 +02:00

204 lines
6.6 KiB
Diff
Raw Blame History

From 7957a25edf844c966de45848fa7e9e2513955660 Mon Sep 17 00:00:00 2001
From: Hiroshi SHIBATA <hsbt@ruby-lang.org>
Date: Thu, 21 Mar 2024 15:47:40 +0900
Subject: [PATCH 1/2] Merge RDoc-6.3.4.1
---
lib/rdoc/rdoc.rb | 3 ++-
lib/rdoc/store.rb | 45 ++++++++++++++++++++--------------
test/rdoc/test_rdoc_options.rb | 6 ++---
3 files changed, 31 insertions(+), 23 deletions(-)
diff --git a/lib/rdoc/rdoc.rb b/lib/rdoc/rdoc.rb
index a2711fbbd1..c5690fc3b4 100644
--- a/lib/rdoc/rdoc.rb
+++ b/lib/rdoc/rdoc.rb
@@ -162,8 +162,9 @@ def load_options
RDoc.load_yaml
begin
- options = YAML.load_file '.rdoc_options'
+ options = YAML.safe_load_file '.rdoc_options', permitted_classes: [RDoc::Options, Symbol]
rescue Psych::SyntaxError
+ raise RDoc::Error, "#{options_file} is not a valid rdoc options file"
end
raise RDoc::Error, "#{options_file} is not a valid rdoc options file" unless
diff --git a/lib/rdoc/store.rb b/lib/rdoc/store.rb
index 999aa76f92..07d03e90f7 100644
--- a/lib/rdoc/store.rb
+++ b/lib/rdoc/store.rb
@@ -539,9 +539,7 @@ def load_all
def load_cache
#orig_enc = @encoding
- open cache_path, 'rb' do |io|
- @cache = Marshal.load io.read
- end
+ @cache = marshal_load(cache_path)
load_enc = @cache[:encoding]
@@ -596,9 +594,7 @@ def load_class klass_name
def load_class_data klass_name
file = class_file klass_name
- open file, 'rb' do |io|
- Marshal.load io.read
- end
+ marshal_load(file)
rescue Errno::ENOENT => e
error = MissingFileError.new(self, file, klass_name)
error.set_backtrace e.backtrace
@@ -611,14 +607,10 @@ def load_class_data klass_name
def load_method klass_name, method_name
file = method_file klass_name, method_name
- open file, 'rb' do |io|
- obj = Marshal.load io.read
- obj.store = self
- obj.parent =
- find_class_or_module(klass_name) || load_class(klass_name) unless
- obj.parent
- obj
- end
+ obj = marshal_load(file)
+ obj.store = self
+ obj.parent ||= find_class_or_module(klass_name) || load_class(klass_name)
+ obj
rescue Errno::ENOENT => e
error = MissingFileError.new(self, file, klass_name + method_name)
error.set_backtrace e.backtrace
@@ -631,11 +623,9 @@ def load_method klass_name, method_name
def load_page page_name
file = page_file page_name
- open file, 'rb' do |io|
- obj = Marshal.load io.read
- obj.store = self
- obj
- end
+ obj = marshal_load(file)
+ obj.store = self
+ obj
rescue Errno::ENOENT => e
error = MissingFileError.new(self, file, page_name)
error.set_backtrace e.backtrace
@@ -965,4 +955,21 @@ def unique_modules
@unique_modules
end
+ private
+ def marshal_load(file)
+ File.open(file, 'rb') {|io| Marshal.load(io, MarshalFilter)}
+ end
+
+ MarshalFilter = proc do |obj|
+ case obj
+ when true, false, nil, Array, Class, Encoding, Hash, Integer, String, Symbol, RDoc::Text
+ else
+ unless obj.class.name.start_with?("RDoc::")
+ raise TypeError, "not permitted class: #{obj.class.name}"
+ end
+ end
+ obj
+ end
+ private_constant :MarshalFilter
+
end
diff --git a/test/rdoc/test_rdoc_options.rb b/test/rdoc/test_rdoc_options.rb
index 400ed9a549..247c7c87ce 100644
--- a/test/rdoc/test_rdoc_options.rb
+++ b/test/rdoc/test_rdoc_options.rb
@@ -145,7 +145,7 @@ def test_init_with_encoding
@options.encoding = Encoding::IBM437
- options = YAML.load YAML.dump @options
+ options = YAML.safe_load(YAML.dump(@options), permitted_classes: [RDoc::Options, Symbol])
assert_equal Encoding::IBM437, options.encoding
end
@@ -161,7 +161,7 @@ def test_init_with_trim_paths
- /etc
YAML
- options = YAML.load yaml
+ options = YAML.safe_load(yaml, permitted_classes: [RDoc::Options, Symbol])
assert_empty options.rdoc_include
assert_empty options.static_path
@@ -729,7 +729,7 @@ def test_write_options
assert File.exist? '.rdoc_options'
- assert_equal @options, YAML.load(File.read('.rdoc_options'))
+ assert_equal @options, YAML.safe_load(File.read('.rdoc_options'), permitted_classes: [RDoc::Options, Symbol])
end
end
From 153a4d16058783c923d0df5b1cbe2610ef96e3a8 Mon Sep 17 00:00:00 2001
From: Jarek Prokop <jprokop@redhat.com>
Date: Tue, 28 May 2024 16:56:26 +0200
Subject: [PATCH 2/2] Port the rebase to work with Ruby 2.5.9.
Ruby 2.5's Psych does not have safe_load_file method.
However, from Ruby 3.3's sources, the method is just File.read
simple wrapper with a safe_load call. Therefore it was copied over to
the lib/rdoc/rdoc.rb file.
---
lib/rdoc/rdoc.rb | 9 ++++++-
test/rdoc/test_rdoc_options.rb | 6 +++---
2 files changed, 11 insertions(+), 4 deletions(-)
diff --git a/lib/rdoc/rdoc.rb b/lib/rdoc/rdoc.rb
index c5690fc3b4..435cd2eaf0 100644
--- a/lib/rdoc/rdoc.rb
+++ b/lib/rdoc/rdoc.rb
@@ -162,7 +162,12 @@ def load_options
RDoc.load_yaml
begin
- options = YAML.safe_load_file '.rdoc_options', permitted_classes: [RDoc::Options, Symbol]
+ # Opening file inspired from Ruby 3.3.0 sources,
+ # file 'ext/psych/lib/psych.rb', line 658.
+ # https://github.com/ruby/ruby/blob/v3_3_0/ext/psych/lib/psych.rb#L658
+ options = File.open('.rdoc_options', 'r:bom|utf-8') do |file|
+ YAML.safe_load file, [RDoc::Options, Symbol], [], false, '.rdoc_options'
+ end
rescue Psych::SyntaxError
raise RDoc::Error, "#{options_file} is not a valid rdoc options file"
end
diff --git a/test/rdoc/test_rdoc_options.rb b/test/rdoc/test_rdoc_options.rb
index 247c7c87ce..60fe035dce 100644
--- a/test/rdoc/test_rdoc_options.rb
+++ b/test/rdoc/test_rdoc_options.rb
@@ -145,7 +145,7 @@ def test_init_with_encoding
@options.encoding = Encoding::IBM437
- options = YAML.safe_load(YAML.dump(@options), permitted_classes: [RDoc::Options, Symbol])
+ options = YAML.safe_load(YAML.dump(@options), [RDoc::Options, Symbol])
assert_equal Encoding::IBM437, options.encoding
end
@@ -161,7 +161,7 @@ def test_init_with_trim_paths
- /etc
YAML
- options = YAML.safe_load(yaml, permitted_classes: [RDoc::Options, Symbol])
+ options = YAML.safe_load(yaml, [RDoc::Options, Symbol])
assert_empty options.rdoc_include
assert_empty options.static_path
@@ -729,7 +729,7 @@ def test_write_options
assert File.exist? '.rdoc_options'
- assert_equal @options, YAML.safe_load(File.read('.rdoc_options'), permitted_classes: [RDoc::Options, Symbol])
+ assert_equal @options, YAML.safe_load(File.read('.rdoc_options'), [RDoc::Options, Symbol])
end
end
Reference in New Issue View Git Blame Copy Permalink