diff mbox series

[kirkstone,1/1] ruby: correct fix for CVE-2024-43398

Message ID 20250712231424.1840000-2-rob.woolley@windriver.com
State New
Headers show
Series ruby: correct fix for CVE-2024-43398 | expand

Commit Message

Rob Woolley July 12, 2025, 11:13 p.m. UTC
The previous fix for CVE-2024-43398 did not include patches
to provide context for the changes it made.

This caused an exception at run-time when ruby parsed
rexml/parsers/baseparser.rb. This was first observed when using
ruby-native to build the sdformat recipe.

With these additional backports, the sdformat build proceeds
successfully. The REXML library was also tested manually on-target
with a script that used REXML::Document.new file to parse an
XML file.

Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
---
 .../ruby/ruby/CVE-2024-43398-0001.patch       | 210 ++++++++++++++++++
 .../ruby/ruby/CVE-2024-43398-0002.patch       | 128 +++++++++++
 ...-43398.patch => CVE-2024-43398-0003.patch} |  23 +-
 meta/recipes-devtools/ruby/ruby_3.1.3.bb      |   4 +-
 4 files changed, 351 insertions(+), 14 deletions(-)
 create mode 100644 meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0001.patch
 create mode 100644 meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0002.patch
 rename meta/recipes-devtools/ruby/ruby/{CVE-2024-43398.patch => CVE-2024-43398-0003.patch} (87%)

Comments

patchtest@automation.yoctoproject.org July 12, 2025, 11:30 p.m. UTC | #1
Thank you for your submission. Patchtest identified one
or more issues with the patch. Please see the log below for
more information:

---
Testing patch /home/patchtest/share/mboxes/kirkstone-1-1-ruby-correct-fix-for-CVE-2024-43398.patch

FAIL: test CVE tag format: Missing or incorrectly formatted CVE tag in patch file. Correct or include the CVE tag in the patch with format: "CVE: CVE-YYYY-XXXX" (test_patch.TestPatch.test_cve_tag_format)

PASS: pretest src uri left files (test_metadata.TestMetadata.pretest_src_uri_left_files)
PASS: test Signed-off-by presence (test_mbox.TestMbox.test_signed_off_by_presence)
PASS: test Signed-off-by presence (test_patch.TestPatch.test_signed_off_by_presence)
PASS: test Upstream-Status presence (test_patch.TestPatch.test_upstream_status_presence_format)
PASS: test author valid (test_mbox.TestMbox.test_author_valid)
PASS: test commit message presence (test_mbox.TestMbox.test_commit_message_presence)
PASS: test commit message user tags (test_mbox.TestMbox.test_commit_message_user_tags)
PASS: test lic files chksum modified not mentioned (test_metadata.TestMetadata.test_lic_files_chksum_modified_not_mentioned)
PASS: test max line length (test_metadata.TestMetadata.test_max_line_length)
PASS: test mbox format (test_mbox.TestMbox.test_mbox_format)
PASS: test non-AUH upgrade (test_mbox.TestMbox.test_non_auh_upgrade)
PASS: test shortlog format (test_mbox.TestMbox.test_shortlog_format)
PASS: test shortlog length (test_mbox.TestMbox.test_shortlog_length)
PASS: test src uri left files (test_metadata.TestMetadata.test_src_uri_left_files)
PASS: test target mailing list (test_mbox.TestMbox.test_target_mailing_list)

SKIP: pretest pylint: No python related patches, skipping test (test_python_pylint.PyLint.pretest_pylint)
SKIP: test CVE check ignore: No modified recipes or older target branch, skipping test (test_metadata.TestMetadata.test_cve_check_ignore)
SKIP: test bugzilla entry format: No bug ID found (test_mbox.TestMbox.test_bugzilla_entry_format)
SKIP: test lic files chksum presence: No added recipes, skipping test (test_metadata.TestMetadata.test_lic_files_chksum_presence)
SKIP: test license presence: No added recipes, skipping test (test_metadata.TestMetadata.test_license_presence)
SKIP: test pylint: No python related patches, skipping test (test_python_pylint.PyLint.test_pylint)
SKIP: test series merge on head: Merge test is disabled for now (test_mbox.TestMbox.test_series_merge_on_head)
SKIP: test summary presence: No added recipes, skipping test (test_metadata.TestMetadata.test_summary_presence)

---

Please address the issues identified and
submit a new revision of the patch, or alternatively, reply to this
email with an explanation of why the patch should be accepted. If you
believe these results are due to an error in patchtest, please submit a
bug at https://bugzilla.yoctoproject.org/ (use the 'Patchtest' category
under 'Yocto Project Subprojects'). For more information on specific
failures, see: https://wiki.yoctoproject.org/wiki/Patchtest. Thank
you!
diff mbox series

Patch

diff --git a/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0001.patch b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0001.patch
new file mode 100644
index 0000000000..2dd6aa6589
--- /dev/null
+++ b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0001.patch
@@ -0,0 +1,210 @@ 
+From 0496940d5998ccbc50d16fb734993ab50fc60c2d Mon Sep 17 00:00:00 2001
+From: NAITOH Jun <naitoh@gmail.com>
+Date: Mon, 18 Mar 2024 23:30:47 +0900
+Subject: [PATCH]  Optimize the parse_attributes method to use `Source#match`
+ to parse XML.  (#119)
+
+## Why?
+
+Improve maintainability by consolidating processing into `Source#match`.
+
+## Benchmark
+```
+RUBYLIB= BUNDLER_ORIG_RUBYLIB= /Users/naitoh/.rbenv/versions/3.3.0/bin/ruby -v -S benchmark-driver /Users/naitoh/ghq/github.com/naitoh/rexml/benchmark/parse.yaml
+ruby 3.3.0 (2023-12-25 revision 5124f9ac75) [arm64-darwin22]
+Calculating -------------------------------------
+                         before       after  before(YJIT)  after(YJIT)
+                 dom     10.891      10.622        16.356       17.403 i/s -     100.000 times in 9.182130s 9.414177s 6.113806s 5.746133s
+                 sax     30.335      29.845        49.749       54.877 i/s -     100.000 times in 3.296483s 3.350595s 2.010071s 1.822259s
+                pull     35.514      34.801        61.123       66.908 i/s -     100.000 times in 2.815793s 2.873484s 1.636041s 1.494591s
+              stream     35.141      34.475        52.110       56.836 i/s -     100.000 times in 2.845646s 2.900638s 1.919017s 1.759456s
+
+Comparison:
+                              dom
+         after(YJIT):        17.4 i/s
+        before(YJIT):        16.4 i/s - 1.06x  slower
+              before:        10.9 i/s - 1.60x  slower
+               after:        10.6 i/s - 1.64x  slower
+
+                              sax
+         after(YJIT):        54.9 i/s
+        before(YJIT):        49.7 i/s - 1.10x  slower
+              before:        30.3 i/s - 1.81x  slower
+               after:        29.8 i/s - 1.84x  slower
+
+                             pull
+         after(YJIT):        66.9 i/s
+        before(YJIT):        61.1 i/s - 1.09x  slower
+              before:        35.5 i/s - 1.88x  slower
+               after:        34.8 i/s - 1.92x  slower
+
+                           stream
+         after(YJIT):        56.8 i/s
+        before(YJIT):        52.1 i/s - 1.09x  slower
+              before:        35.1 i/s - 1.62x  slower
+               after:        34.5 i/s - 1.65x  slower
+
+```
+
+- YJIT=ON : 1.06x - 1.10x faster
+- YJIT=OFF : 0.97x - 0.98x faster
+
+Upstream-Status: Backport [https://github.com/ruby/rexml/commit/0496940d5998ccbc50d16fb734993ab50fc60c2d]
+
+Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
+---
+ lib/rexml/parsers/baseparser.rb | 116 ++++++++++++--------------------
+ test/parse/test_element.rb      |   4 +-
+ test/test_core.rb               |  20 +++++-
+ 3 files changed, 64 insertions(+), 76 deletions(-)
+
+Index: ruby-3.1.3/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
+===================================================================
+--- ruby-3.1.3.orig/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
++++ ruby-3.1.3/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
+@@ -114,7 +114,7 @@ module REXML
+
+       module Private
+         INSTRUCTION_END = /#{NAME}(\s+.*?)?\?>/um
+-        TAG_PATTERN = /((?>#{QNAME_STR}))/um
++        TAG_PATTERN = /((?>#{QNAME_STR}))\s*/um
+         CLOSE_PATTERN = /(#{QNAME_STR})\s*>/um
+         ATTLISTDECL_END = /\s+#{NAME}(?:#{ATTDEF})*\s*>/um
+         NAME_PATTERN = /\s*#{NAME}/um
+@@ -136,7 +136,6 @@ module REXML
+         self.stream = source
+         @listeners = []
+         @entity_expansion_count = 0
+-        @attributes_scanner = StringScanner.new('')
+       end
+
+       def add_listener( listener )
+@@ -635,86 +634,60 @@ module REXML
+       def parse_attributes(prefixes, curr_ns)
+         attributes = {}
+         closed = false
+-        match_data = @source.match(/^(.*?)(\/)?>/um, true)
+-        if match_data.nil?
+-          message = "Start tag isn't ended"
+-          raise REXML::ParseException.new(message, @source)
+-        end
++        while true
++          if @source.match(">", true)
++            return attributes, closed
++          elsif @source.match("/>", true)
++            closed = true
++            return attributes, closed
++          elsif match = @source.match(QNAME, true)
++            name = match[1]
++            prefix = match[2]
++            local_part = match[3]
+
+-        raw_attributes = match_data[1]
+-        closed = !match_data[2].nil?
+-        return attributes, closed if raw_attributes.nil?
+-        return attributes, closed if raw_attributes.empty?
+-
+-        @attributes_scanner.string = raw_attributes
+-        scanner = @attributes_scanner
+-        until scanner.eos?
+-          if scanner.scan(/\s+/)
+-            break if scanner.eos?
+-          end
+-
+-          pos = scanner.pos
+-          while true
+-            break if scanner.scan(ATTRIBUTE_PATTERN)
+-            unless scanner.scan(QNAME)
+-              message = "Invalid attribute name: <#{scanner.rest}>"
+-              raise REXML::ParseException.new(message, @source)
+-            end
+-            name = scanner[0]
+-            unless scanner.scan(/\s*=\s*/um)
++            unless @source.match(/\s*=\s*/um, true)
+               message = "Missing attribute equal: <#{name}>"
+               raise REXML::ParseException.new(message, @source)
+             end
+-            quote = scanner.scan(/['"]/)
+-            unless quote
+-              message = "Missing attribute value start quote: <#{name}>"
+-              raise REXML::ParseException.new(message, @source)
+-            end
+-            unless scanner.scan(/.*#{Regexp.escape(quote)}/um)
+-              match_data = @source.match(/^(.*?)(\/)?>/um, true)
+-              if match_data
+-                scanner << "/" if closed
+-                scanner << ">"
+-                scanner << match_data[1]
+-                scanner.pos = pos
+-                closed = !match_data[2].nil?
+-                next
++            unless match = @source.match(/(['"])(.*?)\1\s*/um, true)
++              if match = @source.match(/(['"])/, true)
++                message =
++                  "Missing attribute value end quote: <#{name}>: <#{match[1]}>"
++                raise REXML::ParseException.new(message, @source)
++              else
++                message = "Missing attribute value start quote: <#{name}>"
++                raise REXML::ParseException.new(message, @source)
+               end
+-              message =
+-                "Missing attribute value end quote: <#{name}>: <#{quote}>"
+-              raise REXML::ParseException.new(message, @source)
+             end
+-          end
+-          name = scanner[1]
+-          prefix = scanner[2]
+-          local_part = scanner[3]
+-          # quote = scanner[4]
+-          value = scanner[5]
+-          if prefix == "xmlns"
+-            if local_part == "xml"
+-              if value != "http://www.w3.org/XML/1998/namespace"
+-                msg = "The 'xml' prefix must not be bound to any other namespace "+
++            value = match[2]
++            if prefix == "xmlns"
++              if local_part == "xml"
++                if value != "http://www.w3.org/XML/1998/namespace"
++                  msg = "The 'xml' prefix must not be bound to any other namespace "+
++                    "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
++                  raise REXML::ParseException.new( msg, @source, self )
++                end
++              elsif local_part == "xmlns"
++                msg = "The 'xmlns' prefix must not be declared "+
+                   "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
+-                raise REXML::ParseException.new( msg, @source, self )
++                raise REXML::ParseException.new( msg, @source, self)
+               end
+-            elsif local_part == "xmlns"
+-              msg = "The 'xmlns' prefix must not be declared "+
+-                "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
+-              raise REXML::ParseException.new( msg, @source, self)
++              curr_ns << local_part
++            elsif prefix
++              prefixes << prefix unless prefix == "xml"
+             end
+-            curr_ns << local_part
+-          elsif prefix
+-            prefixes << prefix unless prefix == "xml"
+-          end
+
+-          if attributes.has_key?(name)
+-            msg = "Duplicate attribute #{name.inspect}"
+-            raise REXML::ParseException.new(msg, @source, self)
+-          end
++            if attributes.has_key?(name)
++              msg = "Duplicate attribute #{name.inspect}"
++              raise REXML::ParseException.new(msg, @source, self)
++            end
+
+-          attributes[name] = value
++            attributes[name] = value
++          else
++            message = "Invalid attribute name: <#{@source.buffer.split(%r{[/>\s]}).first}>"
++            raise REXML::ParseException.new(message, @source)
++          end
+         end
+-        return attributes, closed
+       end
+     end
+   end
diff --git a/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0002.patch b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0002.patch
new file mode 100644
index 0000000000..cd2c15d937
--- /dev/null
+++ b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0002.patch
@@ -0,0 +1,128 @@ 
+From cb158582f18cebb3bf7b3f21f230e2fb17d435aa Mon Sep 17 00:00:00 2001
+From: Sutou Kouhei <kou@clear-code.com>
+Date: Sat, 17 Aug 2024 17:39:14 +0900
+Subject: [PATCH] parser: keep the current namespaces instead of stack of Set
+
+It improves namespace resolution performance for deep element.
+
+Upstream-Status: Backport [https://github.com/ruby/rexml/commit/cb158582f18cebb3bf7b3f21f230e2fb17d435aa]
+
+Signed-off-by: Rob Woolley <rob.woolley@windriver.com>
+
+---
+ lib/rexml/parsers/baseparser.rb | 45 +++++++++++++++++++++++++--------
+ 1 file changed, 35 insertions(+), 10 deletions(-)
+
+Index: ruby-3.1.3/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
+===================================================================
+--- ruby-3.1.3.orig/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
++++ ruby-3.1.3/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
+@@ -152,7 +152,8 @@ module REXML
+         @tags = []
+         @stack = []
+         @entities = []
+-        @nsstack = []
++        @namespaces = {}
++        @namespaces_restore_stack = []
+       end
+
+       def position
+@@ -235,7 +236,6 @@ module REXML
+                 @source.string = "<!DOCTYPE" + @source.buffer
+                 raise REXML::ParseException.new(message, @source)
+               end
+-              @nsstack.unshift(curr_ns=Set.new)
+               name = parse_name(base_error_message)
+               if @source.match(/\s*\[/um, true)
+                 id = [nil, nil, nil]
+@@ -320,7 +320,7 @@ module REXML
+                   val = attdef[4] if val == "#FIXED "
+                   pairs[attdef[0]] = val
+                   if attdef[0] =~ /^xmlns:(.*)/
+-                    @nsstack[0] << $1
++                    @namespaces[$1] = val
+                   end
+                 end
+               end
+@@ -365,7 +365,7 @@ module REXML
+         begin
+           if @source.match("<", true)
+             if @source.match("/", true)
+-              @nsstack.shift
++              @namespaces_restore_stack.pop
+               last_tag = @tags.pop
+               md = @source.match(CLOSE_PATTERN, true)
+               if md and !last_tag
+@@ -411,18 +411,18 @@ module REXML
+               @document_status = :in_element
+               prefixes = Set.new
+               prefixes << md[2] if md[2]
+-              @nsstack.unshift(curr_ns=Set.new)
+-              attributes, closed = parse_attributes(prefixes, curr_ns)
++              push_namespaces_restore
++              attributes, closed = parse_attributes(@prefixes)
+               # Verify that all of the prefixes have been defined
+               for prefix in prefixes
+-                unless @nsstack.find{|k| k.member?(prefix)}
++                unless @namespaces.key?(prefix)
+                   raise UndefinedNamespaceException.new(prefix,@source,self)
+                 end
+               end
+
+               if closed
+                 @closed = tag
+-                @nsstack.shift
++                pop_namespaces_restore
+               else
+                 @tags.push( tag )
+               end
+@@ -512,6 +512,31 @@ module REXML
+       end
+
+       private
++      def add_namespace(prefix, uri)
++        @namespaces_restore_stack.last[prefix] = @namespaces[prefix]
++        if uri.nil?
++          @namespaces.delete(prefix)
++        else
++          @namespaces[prefix] = uri
++        end
++      end
++
++      def push_namespaces_restore
++        namespaces_restore = {}
++        @namespaces_restore_stack.push(namespaces_restore)
++        namespaces_restore
++      end
++
++      def pop_namespaces_restore
++        namespaces_restore = @namespaces_restore_stack.pop
++        namespaces_restore.each do |prefix, uri|
++          if uri.nil?
++            @namespaces.delete(prefix)
++          else
++            @namespaces[prefix] = uri
++          end
++        end
++      end
+
+       def record_entity_expansion
+         @entity_expansion_count += 1
+@@ -631,7 +656,7 @@ module REXML
+         [:processing_instruction, match_data[1], match_data[2]]
+       end
+
+-      def parse_attributes(prefixes, curr_ns)
++      def parse_attributes(prefixes)
+         attributes = {}
+         closed = false
+         while true
+@@ -672,7 +697,7 @@ module REXML
+                   "(http://www.w3.org/TR/REC-xml-names/#ns-decl)"
+                 raise REXML::ParseException.new( msg, @source, self)
+               end
+-              curr_ns << local_part
++              add_namespace(local_part, value)
+             elsif prefix
+               prefixes << prefix unless prefix == "xml"
+             end
diff --git a/meta/recipes-devtools/ruby/ruby/CVE-2024-43398.patch b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0003.patch
similarity index 87%
rename from meta/recipes-devtools/ruby/ruby/CVE-2024-43398.patch
rename to meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0003.patch
index 02dc0a20be..2a48aabbc5 100644
--- a/meta/recipes-devtools/ruby/ruby/CVE-2024-43398.patch
+++ b/meta/recipes-devtools/ruby/ruby/CVE-2024-43398-0003.patch
@@ -47,17 +47,17 @@  diff --git a/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb b/.bundle/
 index e32c7f4..154f2ac 100644
 --- a/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
 +++ b/.bundle/gems/rexml-3.2.5/lib/rexml/parsers/baseparser.rb
-@@ -634,6 +634,7 @@ module REXML
+@@ -658,6 +658,7 @@ module REXML
  
-       def parse_attributes(prefixes, curr_ns)
+       def parse_attributes(prefixes)
          attributes = {}
 +        expanded_names = {}
          closed = false
-         match_data = @source.match(/^(.*?)(\/)?>/um, true)
-         if match_data.nil?
-@@ -641,6 +642,20 @@ module REXML
-           raise REXML::ParseException.new(message, @source)
-         end
+         while true
+           if @source.match(">", true)
+@@ -707,6 +708,20 @@ module REXML
+               raise REXML::ParseException.new(msg, @source, self)
+             end
  
 +            unless prefix == "xmlns"
 +              uri = @namespaces[prefix]
@@ -73,9 +73,6 @@  index e32c7f4..154f2ac 100644
 +              expanded_names[expanded_name] = prefix
 +            end
 +
-         raw_attributes = match_data[1]
-         closed = !match_data[2].nil?
-         return attributes, closed if raw_attributes.nil?
--- 
-2.40.0
-
+             attributes[name] = value
+           else
+             message = "Invalid attribute name: <#{@source.buffer.split(%r{[/>\s]}).first}>"
diff --git a/meta/recipes-devtools/ruby/ruby_3.1.3.bb b/meta/recipes-devtools/ruby/ruby_3.1.3.bb
index 65d62002ec..19641e5a51 100644
--- a/meta/recipes-devtools/ruby/ruby_3.1.3.bb
+++ b/meta/recipes-devtools/ruby/ruby_3.1.3.bb
@@ -48,7 +48,9 @@  SRC_URI = "http://cache.ruby-lang.org/pub/ruby/${SHRT_VER}/ruby-${PV}.tar.gz \
            file://CVE-2024-41946.patch \
            file://CVE-2025-27220.patch \
            file://CVE-2025-27219.patch \
-           file://CVE-2024-43398.patch \
+           file://CVE-2024-43398-0001.patch \
+           file://CVE-2024-43398-0002.patch \
+           file://CVE-2024-43398-0003.patch \
            file://CVE-2025-27221-0001.patch \
            file://CVE-2025-27221-0002.patch \
            "