`` in SAML response not sanitized before validation

ichi234 commented 1 month ago

Summary

When validating a SAML response, if a newline in the response is  instead of , it is not sanitized and the validation fails.

Environment in which the event occurred

Browser: Microsoft Edge on Windows 10
Execution environment: Next.js application on WSL

Related Library Versions

"@boxyhq/saml20": "1.5.1"
"@boxyhq/saml-jackson": "1.26.1"
"next-auth": "4.24.7"

Detailed Situation

In the case in issue, the X509Certificate portion of the xml argument passed to the validateSignature.hasValidSignature method represented line breaks as . This representation is not sanitized before use and therefore fails validation.

Actual response data (excerpts)

<KeyInfo><X509Data><X509Certificate>MIID2TCCAsGgAwIBAgIBAzANBgkqhkiG9w0BAQsFADBvMQswCQYDVQQGEwJKUDERMA8GA1UECAwI&#13;
SE9ra2FpZG8xFDASBgNVBAcMC1NBUFBPUk8tU0hJMSowKAYDVQQKDCFIb2trYWlkbyBFbGVjdHJp&#13;
YyBQb3dlciBDby4sIEluYy4xCzAJBgNVBAMMAmNhMCAXDTIxMDMwMzAwMzgyMVoYDzIwNTEwMjI0&#13;
MDAzODIxWjByMQswCQYDVQQGEwJKUDERMA8GA1UECAwISE9ra2FpZG8xKjAoBgNVBAoMIUhva2th&#13;
...
vs1WiMb2KXjA5d/xGzkvmxl9Ng6Ama6p3sg38AaLcB72WCNRo5PSYL3aRta5GyfaIAeohJk1Lne+&#13;
neTrBOIBI/dbYuEe9oHPr/IW9fffE4X0iGqBVMy4UzMxIM/DNEmcp7ixVm/u6ybCDbGVWBezSVFy&#13;
SBDO6kbSaR5BAQlvMoF9+3Wixyv7Q3JomUDe7nnp2JrSvh2T1+8tFQWkEa9/1JkJT3O6XrBeY8Me&#13;
n4P9U4e8TThs3VH1lBRV8T1NYUs=</X509Certificate></X509Data></KeyInfo>

Proposed Solution

By manually modifying the contents under node_modules for testing, the following adjustment was found to resolve the issue:

Original code (problematic part): https://github.com/boxyhq/saml20/blob/main/lib/validateSignature.ts#L26
```
xml = xml.replace(/&#x(d|D);/gi, '');
```

Suggested modification:

xml = xml.replace(/(&#x(d|D);|&#13;)/gi, '');

This modification ensures that  is appropriately sanitized, allowing the validation to proceed correctly.

rileyw commented 3 weeks ago

@ichi234 Would you check that this test case matches the SAML response that you have observed to be problematic? https://github.com/rileyw/saml20/blob/issue/599/test/assets/saml20.validResponseSignedMessage-unsanitized.xml#L7-L19

ichi234 commented 3 weeks ago

@rileyw Yes, it matches. The content of the X509Certificate element is broken every 76 characters, and the CRLF at the line break is converted as follows.

CR -> 
LF -> as is

To put a little more detail into it, the value portion of a newline is formatted without line indentation as follows.

<?xml version="1.0" encoding="UTF-8" standalone="no"?><samlp:Response xmlns="urn:oasis:names:tc:SAML:2.0:assertion" xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:xenc="http://www.w3.org/2001/04/xmlenc#" ID="iCznFHxFh9cF3HIwkqxL7kxRI4UbiVU9pCaZGX6R9" IssueInstant="2024-06-07T00:26:59Z" Version="2.0">
  <Issuer>https://example.com/</Issuer>
  <Signature xmlns="http://www.w3.org/2000/09/xmldsig#"><SignedInfo><CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/><SignatureMethod Algorithm="http://www.w3.org/2001/04/xmldsig-more#rsa-sha256"/><Reference URI="#iCznFHxFh9cF3HIwkqxL7kxRI4UbiVU9pCaZGX6R9"><Transforms><Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></Transforms><DigestMethod Algorithm="http://www.w3.org/2001/04/xmlenc#sha256"/><DigestValue>RkCumiTsjLOTY3zpqUs7I4p7IZVPtnRNokk8UhIlRzM=</DigestValue></Reference></SignedInfo><SignatureValue>WJoBk4I5H8QwfP66IxFbPnUK5Dinv8tRfyeRodIS0xcCdTK+tR984zSRe27ihtM3wEih0vjT3Dd3&#13;
D4Q5DldbtQXCW7urS69XG7+CgskO8WCKyoZJXr3/sS4tS5FGHtE9Ef9H0j07L9ag2Z/SmSgGXhOT&#13;
...
ylzVGe6f3zs659YWI0XLRWpBuhktYO3NlZSmHmGo+r+IRZY7/lG30rS+VTqWXQNnRxqwlO8iv5sm&#13;
go46r6rpaAY27fr/E8xb00jbQGKELU/HTLnuVg==</SignatureValue><KeyInfo><X509Data><X509Certificate>MIID2TCCAsGgAwIBAgIBAzANBgkqhkiG9w0BAQsFADBvMQswCQYDVQQGEwJKUDERMA8GA1UECAwI&#13;
SE9ra2FpZG8xFDASBgNVBAcMC1NBUFBPUk8tU0hJMSowKAYDVQQKDCFIb2trYWlkbyBFbGVjdHJp&#13;
YyBQb3dlciBDby4sIEluYy4xCzAJBgNVBAMMAmNhMCAXDTIxMDMwMzAwMzgyMVoYDzIwNTEwMjI0&#13;
...
neTrBOIBI/dbYuEe9oHPr/IW9fffE4X0iGqBVMy4UzMxIM/DNEmcp7ixVm/u6ybCDbGVWBezSVFy&#13;
SBDO6kbSaR5BAQlvMoF9+3Wixyv7Q3JomUDe7nnp2JrSvh2T1+8tFQWkEa9/1JkJT3O6XrBeY8Me&#13;
n4P9U4e8TThs3VH1lBRV8T1NYUs=</X509Certificate></X509Data></KeyInfo></Signature><samlp:Status>
    <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
  </samlp:Status>
  <Assertion ID=...

boxyhq / saml20