From 46b6a33dbaa1c11e8cb98cf10a20b1e9b2f86b59 Mon Sep 17 00:00:00 2001 From: Jarek Prokop Date: Thu, 30 May 2024 20:21:13 +0200 Subject: [PATCH] Fix RCE vulnerability with .rdoc_options in RDoc (CVE-2024-27281). The patch contains 2 commits, 1 from upstream that comes from https://github.com/ruby/ruby/commit/7957a25edf844c966de45848fa7e9e2513955660 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: https://github.com/ruby/psych/commit/0210e310d04cbc9b236ccdde6caaf79aab4eb794#diff-659eac8589abc82c9a0ab3699e4e4be4774d9c09c6c9934af5d9dae0d264439cR592 In Ruby 3.0 this capability was added with this commit: https://github.com/ruby/ruby/commit/c2a60fec2f79c05bdb865c143b6ad8eddfc6cc36#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 https://github.com/ruby/psych/commit/682abf20f0ddbc8d83e816022953536dad80c05b 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: https://github.com/ruby/ruby/commit/7957a25edf844c966de45848fa7e9e2513955660#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 --- ...-RCE-vulnerability-with-rdoc_options.patch | 203 ++++++++++++++++++ ruby.spec | 8 + 2 files changed, 211 insertions(+) create mode 100644 ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch diff --git a/ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch b/ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch new file mode 100644 index 0000000..9816d13 --- /dev/null +++ b/ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch @@ -0,0 +1,203 @@ +From 7957a25edf844c966de45848fa7e9e2513955660 Mon Sep 17 00:00:00 2001 +From: Hiroshi SHIBATA +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 +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 + diff --git a/ruby.spec b/ruby.spec index d2688c6..d0a479e 100644 --- a/ruby.spec +++ b/ruby.spec @@ -243,6 +243,10 @@ Patch44: ruby-3.0.7-Fix-CVE-2023-36617-Upstreams-incomplete-fix-for-CVE-2023-287 # Backported from: # https://github.com/ruby/ruby/commit/bd9424c71c15896a997d5a092bf5e1ed453defa6 Patch45: ruby-3.0.7-Fix-CVE-2024-27280-Buffer-overread-in-StringIO.patch +# CVE-2024-27281 RCE vulnerability with .rdoc_options in RDoc. +# Backported from: +# https://github.com/ruby/ruby/commit/7957a25edf844c966de45848fa7e9e2513955660 +Patch46: ruby-3.0.7-Fix-CVE-2024-27281-RCE-vulnerability-with-rdoc_options.patch Requires: %{name}-libs%{?_isa} = %{version}-%{release} @@ -659,6 +663,7 @@ sed -i 's/"evaluation\/incorrect_words.yaml"\.freeze, //' \ %patch43 -p1 %patch44 -p1 %patch45 -p1 +%patch46 -p1 # Provide an example of usage of the tapset: cp -a %{SOURCE3} . @@ -1218,6 +1223,9 @@ OPENSSL_SYSTEM_CIPHERS_OVERRIDE=xyz_nonexistent_file OPENSSL_CONF='' \ - Fix Buffer overread vulnerability in StringIO. (CVE-2024-27280) Resolves: RHEL-34125 +- Fix RCE vulnerability with .rdoc_options in RDoc. + (CVE-2024-27281) + Resolves: RHEL-34117 * Mon Jun 12 2023 Jarek Prokop - 2.5.9-111 - Fix HTTP response splitting in CGI.