Fix vulnerabilities: CVE-2025-25291, CVE-2025-25292: SAML authentication bypass via Signature Wrapping attack allowed due parser differential

Author: pitbulk

lib/onelogin/ruby-saml/logoutresponse.rb

@@ -150,7 +150,8 @@ def validate_success_status
       # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_structure
-        unless valid_saml?(document, soft)
+        check_malformed_doc = check_malformed_doc?(settings)
+        unless valid_saml?(document, soft, check_malformed_doc)
           return append_error("Invalid SAML Logout Response. Not match the saml-schema-protocol-2.0.xsd")
         end
 

lib/onelogin/ruby-saml/response.rb

@@ -425,12 +425,13 @@ def validate_success_status
       #
       def validate_structure
         structure_error_msg = "Invalid SAML Response. Not match the saml-schema-protocol-2.0.xsd"
-        unless valid_saml?(document, soft)
+        check_malformed_doc = check_malformed_doc_enabled?
+        unless valid_saml?(document, soft, check_malformed_doc)
           return append_error(structure_error_msg)
         end
 
         unless decrypted_document.nil?
-          unless valid_saml?(decrypted_document, soft)
+          unless valid_saml?(decrypted_document, soft, check_malformed_doc)
             return append_error(structure_error_msg)
           end
         end
@@ -865,6 +866,8 @@ def validate_signature
           fingerprint = settings.get_fingerprint
           opts[:cert] = idp_cert
 
+          check_malformed_doc = check_malformed_doc_enabled?
+          opts[:check_malformed_doc] = check_malformed_doc
           if fingerprint && doc.validate_document(fingerprint, @soft, opts)
             if settings.security[:check_idp_cert_expiration]
               if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
@@ -879,7 +882,7 @@ def validate_signature
           valid = false
           expired = false
           idp_certs[:signing].each do |idp_cert|
-            valid = doc.validate_document_with_cert(idp_cert, true)
+            valid = doc.validate_document_with_cert(idp_cert, true, check_malformed_doc)
             if valid
               if settings.security[:check_idp_cert_expiration]
                 if OneLogin::RubySaml::Utils.is_cert_expired(idp_cert)
@@ -1063,6 +1066,10 @@ def parse_time(node, attribute)
           Time.parse(node.attributes[attribute])
         end
       end
+
+      def check_malformed_doc_enabled?
+        check_malformed_doc?(settings)
+      end
     end
   end
 end

lib/onelogin/ruby-saml/saml_message.rb

@@ -63,14 +63,13 @@ def id(document)
       # Validates the SAML Message against the specified schema.
       # @param document [REXML::Document] The message that will be validated
       # @param soft [Boolean] soft Enable or Disable the soft mode (In order to raise exceptions when the message is invalid or not)
+      # @param check_malformed_doc [Boolean] check_malformed_doc Enable or Disable the check for malformed XML
       # @return [Boolean] True if the XML is valid, otherwise False, if soft=True
       # @raise [ValidationError] if soft == false and validation fails
       #
-      def valid_saml?(document, soft = true)
+      def valid_saml?(document, soft = true, check_malformed_doc = true)
         begin
-          xml = Nokogiri::XML(document.to_s) do |config|
-            config.options = XMLSecurity::BaseDocument::NOKOGIRI_OPTIONS
-          end
+          xml = XMLSecurity::BaseDocument.safe_load_xml(document, check_malformed_doc)
         rescue Exception => error
           return false if soft
           raise ValidationError.new("XML load failed: #{error.message}")
@@ -163,6 +162,12 @@ def inflate(deflated)
       def deflate(inflated)
         Zlib::Deflate.deflate(inflated, 9)[2..-5]
       end
+
+      def check_malformed_doc?(settings)
+        default_value = OneLogin::RubySaml::Settings::DEFAULTS[:check_malformed_doc]
+
+        settings.nil? ? default_value : settings.check_malformed_doc
+      end
     end
   end
 end

lib/onelogin/ruby-saml/settings.rb

@@ -53,6 +53,7 @@ def initialize(overrides = {}, keep_security_attributes = false)
       attr_accessor :compress_request
       attr_accessor :compress_response
       attr_accessor :double_quote_xml_attribute_values
+      attr_accessor :check_malformed_doc
       attr_accessor :passive
       attr_accessor :protocol_binding
       attr_accessor :attributes_index
@@ -259,6 +260,7 @@ def get_sp_key
         :compress_request                          => true,
         :compress_response                         => true,
         :soft                                      => true,
+        :check_malformed_doc                       => true, 
         :double_quote_xml_attribute_values         => false,
         :security                                  => {
           :authn_requests_signed      => false,

lib/onelogin/ruby-saml/slo_logoutrequest.rb

@@ -199,7 +199,8 @@ def validate_not_on_or_after
       # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_structure
-        unless valid_saml?(document, soft)
+        check_malformed_doc = check_malformed_doc?(settings)
+        unless valid_saml?(document, soft, check_malformed_doc)
           return append_error("Invalid SAML Logout Request. Not match the saml-schema-protocol-2.0.xsd")
         end
 

lib/xml_security.rb

@@ -42,6 +42,36 @@ class BaseDocument < REXML::Document
     NOKOGIRI_OPTIONS = Nokogiri::XML::ParseOptions::STRICT |
                        Nokogiri::XML::ParseOptions::NONET
 
+    # Safety load the SAML Message XML
+    # @param document [REXML::Document] The message to be loaded
+    # @param check_malformed_doc [Boolean] check_malformed_doc Enable or Disable the check for malformed XML
+    # @return [Nokogiri::XML] The nokogiri document
+    # @raise [ValidationError] If there was a problem loading the SAML Message XML
+    def self.safe_load_xml(document, check_malformed_doc = true)
+      doc_str = document.to_s
+      if doc_str.include?("<!DOCTYPE")
+       raise StandardError.new("Dangerous XML detected. No Doctype nodes allowed")
+      end
+
+      begin
+        xml = Nokogiri::XML(doc_str) do |config|
+          config.options = self::NOKOGIRI_OPTIONS
+        end
+      rescue StandardError => error
+        raise StandardError.new(error.message)
+      end
+
+      if xml.internal_subset
+        raise StandardError.new("Dangerous XML detected. No Doctype nodes allowed")
+      end
+
+      unless xml.errors.empty?
+        raise StandardError.new("There were XML errors when parsing: #{xml.errors}") if check_malformed_doc
+      end
+
+      xml
+    end
+
     def canon_algorithm(element)
       algorithm = element
       if algorithm.is_a?(REXML::Element)
@@ -114,10 +144,8 @@ def uuid
       #<KeyInfo />
       #<Object />
     #</Signature>
-    def sign_document(private_key, certificate, signature_method = RSA_SHA1, digest_method = SHA1)
-      noko = Nokogiri::XML(self.to_s) do |config|
-        config.options = XMLSecurity::BaseDocument::NOKOGIRI_OPTIONS
-      end
+    def sign_document(private_key, certificate, signature_method = RSA_SHA1, digest_method = SHA1, check_malformed_doc = true)
+      noko = XMLSecurity::BaseDocument.safe_load_xml(self.to_s, check_malformed_doc)
 
       signature_element = REXML::Element.new("ds:Signature").add_namespace('ds', DSIG)
       signed_info_element = signature_element.add_element("ds:SignedInfo")
@@ -139,9 +167,7 @@ def sign_document(private_key, certificate, signature_method = RSA_SHA1, digest_
       reference_element.add_element("ds:DigestValue").text = compute_digest(canon_doc, algorithm(digest_method_element))
 
       # add SignatureValue
-      noko_sig_element = Nokogiri::XML(signature_element.to_s) do |config|
-        config.options = XMLSecurity::BaseDocument::NOKOGIRI_OPTIONS
-      end
+      noko_sig_element = XMLSecurity::BaseDocument.safe_load_xml(signature_element.to_s, check_malformed_doc)
 
       noko_signed_info_element = noko_sig_element.at_xpath('//ds:Signature/ds:SignedInfo', 'ds' => DSIG)
       canon_string = noko_signed_info_element.canonicalize(canon_algorithm(C14N))
@@ -237,10 +263,12 @@ def validate_document(idp_cert_fingerprint, soft = true, options = {})
           end
         end
       end
-      validate_signature(base64_cert, soft)
+      check_malformed_doc = true
+      check_malformed_doc = options[:check_malformed_doc] if options.key?(:check_malformed_doc)
+      validate_signature(base64_cert, soft, check_malformed_doc)
     end
 
-    def validate_document_with_cert(idp_cert, soft = true)
+    def validate_document_with_cert(idp_cert, soft = true, check_malformed_doc = true)
       # get cert from response
       cert_element = REXML::XPath.first(
         self,
@@ -264,13 +292,17 @@ def validate_document_with_cert(idp_cert, soft = true)
       else
         base64_cert = Base64.encode64(idp_cert.to_pem)
       end
-      validate_signature(base64_cert, true)
+      validate_signature(base64_cert, true, check_malformed_doc)
     end
 
-    def validate_signature(base64_cert, soft = true)
+    def validate_signature(base64_cert, soft = true, check_malformed_doc = true)
 
-      document = Nokogiri::XML(self.to_s) do |config|
-        config.options = XMLSecurity::BaseDocument::NOKOGIRI_OPTIONS
+      begin
+        document = XMLSecurity::BaseDocument.safe_load_xml(self, check_malformed_doc)
+      rescue StandardError => error
+        @errors << error.message
+        return false if soft
+        raise ValidationError.new("XML load failed: #{error.message}")
       end
 
       # create a rexml document