Skip to content

Remove redundant REXML pre-parse in Saml::Base.parse as nokogiri blocks entity expansion natively #194

Description

@baala3

Problem

lib/saml/base.rb runs every inbound XML string through REXML before the real Nokogiri parse:

def parse(xml, options = {})
  if xml.is_a?(String)
    ActiveSupport::XmlMini_REXML.parse(xml)   # return value discarded
  end
  object = super   # actual parse, via Nokogiri (through xmlmapper)

This was needed fix in 2014 (db1ef5b8) for "billion laughs" entity-expansion DoS. At that time Nokogiri didn't protect against it and REXML did, so REXML ran first as a tripwire. This issue also came up before in #171 and was closed as intentional. Revisiting this because libxml2's protections have changed since.

What I verified

I verified billion-laughs fixture from spec/lib/saml/base_spec.rb directly into Nokogiri (version 1.19.4), using Nokogiri::XML::ParseOptions::STRICT (the exact call super makes). This bypasses REXML entirely:

Nokogiri::XML::SyntaxError: FATAL: Maximum entity amplification factor exceeded,
see xmlCtxtSetMaxAmplification.

libxml2 refuses the payload on its own now, with no memory growth. Nokogiri::XML::SyntaxError is already rescued by base.rb and wrapped into Saml::Errors::UnparseableMessage.

Also, current REXML guard also raises RuntimeError, which base.rb's rescue clause doesn't catch (it only lists Nokogiri::XML::SyntaxError, REXML::ParseException, TypeError, NoMethodError). This pattern leaks undocumented raw exception instead of the library's usual Saml::Errors::UnparseableMessage. Removing REXML fixes this issue also.

Why it's safe

  • General-entity billion laughs: blocked by libxml2's entity-expansion ceiling since 2.9.2 (2014).
  • Parameter-entity bypass ("Parameter Laughs", CVE-2021-3541): fixed in libxml2 2.9.11, absorbed by Nokogiri 1.11.4 (GHSA-7rrm-v45f-jp64).
  • The current gemspec (nokogiri '~> 1.11') permits 1.11.0–1.11.3, versions that predate that fix. This needs a floor bump too.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions