zendesk / samlr

Clean room implementation of SAML for Ruby
Apache License 2.0
30 stars 12 forks source link

adding test for xml comment attack #27

Closed swatikri closed 6 years ago

swatikri commented 6 years ago

/cc @zendesk/secdev @kintner

Description

[JUST A TEST so not bumping anywhere] This PR adds a small test to ensure that samlr is not vulnerable to an XML comment parsing attack. Problem: Some SAML implementations parse assertions such as <NameID>user@user.com.evil.com</NameID> and <NameID>user@user.com<!---->.evil.com</NameID> as user@user.com.evil.com and user@user.com (ignoring the part after the comment) respectively. However, both would have identical signatures as XML canonicalization mostly removes comments as part of signature validation. This would allow an attacker to sneak a comment into a SAML assertion and effectively log in as user@user.com without failing signature checks. This PR simply adds tests that ensure -

  1. adding a comment in the SAML assertion still validates the signature correctly.
  2. adding a comment in the SAML assertion does not just give the first part, but the whole identifier.

Risks