Security fixes

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

@@ -17,6 +17,10 @@ class Response < SamlMessage
       PROTOCOL  = "urn:oasis:names:tc:SAML:2.0:protocol"
       DSIG      = "http://www.w3.org/2000/09/xmldsig#"
       XENC      = "http://www.w3.org/2001/04/xmlenc#"
+      SAML_NAMESPACES = {
+        "p" => PROTOCOL,
+        "a" => ASSERTION
+      }.freeze
 
       # TODO: Settings should probably be initialized too... WDYT?
 
@@ -303,7 +307,7 @@ def issuers
           issuer_response_nodes = REXML::XPath.match(
             document,
             "/p:Response/a:Issuer",
-            { "p" => PROTOCOL, "a" => ASSERTION }
+            SAML_NAMESPACES
           )
 
           unless issuer_response_nodes.size == 1
@@ -370,7 +374,7 @@ def assertion_encrypted?
         ! REXML::XPath.first(
           document,
           "(/p:Response/EncryptedAssertion/)|(/p:Response/a:EncryptedAssertion/)",
-          { "p" => PROTOCOL, "a" => ASSERTION }
+          SAML_NAMESPACES
         ).nil?
       end
 
@@ -401,9 +405,9 @@ def validate(collect_errors = false)
           :validate_id,
           :validate_success_status,
           :validate_num_assertion,
-          :validate_no_duplicated_attributes,
           :validate_signed_elements,
           :validate_structure,
+          :validate_no_duplicated_attributes,
           :validate_in_response_to,
           :validate_one_conditions,
           :validate_conditions,
@@ -444,12 +448,14 @@ 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
@@ -841,31 +847,47 @@ def validate_name_id
         true
       end
 
+      def doc_to_validate
+        # If the response contains the signature, and the assertion was encrypted, validate the original SAML Response
+        # otherwise, review if the decrypted assertion contains a signature
+        sig_elements = REXML::XPath.match(
+          document,
+          "/p:Response[@ID=$id]/ds:Signature",
+          { "p" => PROTOCOL, "ds" => DSIG },
+          { 'id' => document.signed_element_id }
+        )
+
+        use_original = sig_elements.size == 1 || decrypted_document.nil?
+        doc = use_original ? document : decrypted_document
+        if !doc.processed
+          doc.cache_referenced_xml(@soft, check_malformed_doc_enabled?)
+        end
+
+        return doc
+      end
+
       # Validates the Signature
       # @return [Boolean] True if not contains a Signature or if the Signature is valid, otherwise False if soft=True
       # @raise [ValidationError] if soft == false and validation fails
       #
       def validate_signature
         error_msg = "Invalid Signature on SAML Response"
 
-        # If the response contains the signature, and the assertion was encrypted, validate the original SAML Response
-        # otherwise, review if the decrypted assertion contains a signature
+        doc = doc_to_validate
+
         sig_elements = REXML::XPath.match(
           document,
           "/p:Response[@ID=$id]/ds:Signature",
           { "p" => PROTOCOL, "ds" => DSIG },
           { 'id' => document.signed_element_id }
         )
 
-        use_original = sig_elements.size == 1 || decrypted_document.nil?
-        doc = use_original ? document : decrypted_document
-
-        # Check signature nodes
+        # Check signature node inside assertion
         if sig_elements.nil? || sig_elements.size == 0
           sig_elements = REXML::XPath.match(
             doc,
             "/p:Response/a:Assertion[@ID=$id]/ds:Signature",
-            {"p" => PROTOCOL, "a" => ASSERTION, "ds"=>DSIG},
+            SAML_NAMESPACES.merge({"ds"=>DSIG}),
             { 'id' => doc.signed_element_id }
           )
         end
@@ -943,24 +965,47 @@ def name_id_node
           end
       end
 
+      def get_cached_signed_assertion
+        xml = doc_to_validate.referenced_xml
+        empty_doc = REXML::Document.new
+
+        return empty_doc if xml.nil? # when no signature/reference is found, return empty document
+
+        root = REXML::Document.new(xml).root
+
+        if root.attributes["ID"] != doc_to_validate.signed_element_id
+          return empty_doc
+        end
+
+        assertion = empty_doc
+        if root.name == "Response"
+          if REXML::XPath.first(root, "a:Assertion", {"a" => ASSERTION})
+            assertion = REXML::XPath.first(root, "a:Assertion", {"a" => ASSERTION})
+          elsif REXML::XPath.first(root, "a:EncryptedAssertion", {"a" => ASSERTION})
+            assertion = decrypt_assertion(REXML::XPath.first(root, "a:EncryptedAssertion", {"a" => ASSERTION}))
+          end
+        elsif root.name == "Assertion"
+          assertion = root
+        end
+
+        assertion
+      end
+
+      def signed_assertion
+        @signed_assertion ||= get_cached_signed_assertion
+      end
+
       # Extracts the first appearance that matchs the subelt (pattern)
       # Search on any Assertion that is signed, or has a Response parent signed
       # @param subelt [String] The XPath pattern
       # @return [REXML::Element | nil] If any matches, return the Element
       #
       def xpath_first_from_signed_assertion(subelt=nil)
-        doc = decrypted_document.nil? ? document : decrypted_document
+        doc = signed_assertion
         node = REXML::XPath.first(
             doc,
-            "/p:Response/a:Assertion[@ID=$id]#{subelt}",
-            { "p" => PROTOCOL, "a" => ASSERTION },
-            { 'id' => doc.signed_element_id }
-        )
-        node ||= REXML::XPath.first(
-            doc,
-            "/p:Response[@ID=$id]/a:Assertion#{subelt}",
-            { "p" => PROTOCOL, "a" => ASSERTION },
-            { 'id' => doc.signed_element_id }
+            "./#{subelt}",
+            SAML_NAMESPACES
         )
         node
       end
@@ -971,19 +1016,13 @@ def xpath_first_from_signed_assertion(subelt=nil)
       # @return [Array of REXML::Element] Return all matches
       #
       def xpath_from_signed_assertion(subelt=nil)
-        doc = decrypted_document.nil? ? document : decrypted_document
+        doc = signed_assertion
         node = REXML::XPath.match(
             doc,
-            "/p:Response/a:Assertion[@ID=$id]#{subelt}",
-            { "p" => PROTOCOL, "a" => ASSERTION },
-            { 'id' => doc.signed_element_id }
+            "./#{subelt}",
+            SAML_NAMESPACES
         )
-        node.concat( REXML::XPath.match(
-            doc,
-            "/p:Response[@ID=$id]/a:Assertion#{subelt}",
-            { "p" => PROTOCOL, "a" => ASSERTION },
-            { 'id' => doc.signed_element_id }
-        ))
+        node
       end
 
       # Generates the decrypted_document
@@ -1017,7 +1056,7 @@ def decrypt_assertion_from_document(document_copy)
         encrypted_assertion_node = REXML::XPath.first(
           document_copy,
           "(/p:Response/EncryptedAssertion/)|(/p:Response/a:EncryptedAssertion/)",
-          { "p" => PROTOCOL, "a" => ASSERTION }
+          SAML_NAMESPACES
         )
         response_node.add(decrypt_assertion(encrypted_assertion_node))
         encrypted_assertion_node.remove
@@ -1087,6 +1126,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

@@ -58,14 +58,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 StandardError => error
           return false if soft
           raise ValidationError.new("XML load failed: #{error.message}")
@@ -81,6 +80,7 @@ def valid_saml?(document, soft = true)
 
       # Base64 decode and try also to inflate a SAML Message
       # @param saml [String] The deflated and encoded SAML Message
+      # @param settings [OneLogin::RubySaml::Settings|nil] Toolkit settings
       # @return [String] The plain SAML Message
       #
       def decode_raw_saml(saml, settings = nil)
@@ -93,10 +93,16 @@ def decode_raw_saml(saml, settings = nil)
 
         decoded = decode(saml)
         begin
-          inflate(decoded)
+            message = inflate(decoded)
         rescue
-          decoded
+            message = decoded
+        end
+
+        if message.bytesize > settings.message_max_bytesize
+            raise ValidationError.new("SAML Message exceeds " + settings.message_max_bytesize.to_s + " bytes, so was rejected")
         end
+
+        message
       end
 
       # Deflate, base64 encode and url-encode a SAML Message (To be used in the HTTP-redirect binding)
@@ -153,6 +159,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

@@ -55,6 +55,7 @@ def initialize(overrides = {}, keep_security_attributes = false)
       attr_accessor :compress_response
       attr_accessor :double_quote_xml_attribute_values
       attr_accessor :message_max_bytesize
+      attr_accessor :check_malformed_doc
       attr_accessor :passive
       attr_reader   :protocol_binding
       attr_accessor :attributes_index
@@ -281,7 +282,9 @@ def get_binding(value)
         :compress_response                         => true,
         :message_max_bytesize                      => 250000,
         :soft                                      => true,
+        :check_malformed_doc                       => true,
         :double_quote_xml_attribute_values         => false,
+
         :security                                  => {
           :authn_requests_signed      => false,
           :logout_requests_signed     => false,

lib/onelogin/ruby-saml/slo_logoutrequest.rb

@@ -238,7 +238,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