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
46b6a33dba