[BUG] XML-encoded carriage returns are not resolved correctly

[mhassan1]

Background

Earlier this week, Google rolled out a change to their GSuite SAML Apps IdP where the SAMLResponse contains XML-encoded carriage returns inside the SignatureValue and X509Certificate blocks.

For example, they started sending this (notice the 
 at the end of each line):

<ds:SignatureValue>nLcgyGLUW61YKksmQ1zc/WlV4/iOVlNWZrT13wUWxMwarc38L0Z1b72gTZjfYwH6t06qm4bNvTKc&#13;
B27ctMgEFvnEBwRuYUluLdhUm1HRPVtKflypI3HDqNtv4wYhbxhL11Yqa/Qhg/cV/VvYA1zMpNax&#13;
SEk10RTllBLmKrQbkmED9vnS1184IkynvpA8rJNqvkwkjVAhQKFIOLr0GVuhgIKP6obTbgtJmXlQ&#13;
Ie2oXsrIegeQqSTk64qsDG5MwG/wm5/26+IBC9Dn8aobLQ798xMZmoXNMyMMfUDK6cbMb2LK3hDR&#13;
NiiwuVSaBboGWTovciv+ui+odUbKHpLhVzJT4g==</ds:SignatureValue>

At first, I thought this must be a bug introduced on their end, but I saw that both samltool.com and SimpleSAMLphp considered that to be a valid SAMLResponse. I also noticed that other Java-based (Apache Santuario) IdPs will provide a SAMLResponse like that when a certain system property is unset (link). I could not find anywhere in the SAML or XML spec that mentions this, but I did notice that DOMParser correctly replaces those 
 with actual carriage returns. We can confirm it like this:

const { DOMParser } = require('xmldom')
new DOMParser()
  .parseFromString('<abc>123&#13;456</abc>')
  .toString()
  .replace(/\r/g, '\\r') // just for display purposes here
// -> `123\r456`

I believe the root of the problem is that validateSignatureForCert passes raw XML (that has never been passed through a DOMParser) to sig.checkSignature(fullXml).

To Reproduce

Add a 
 inside the SignatureValue in test/static/signatures/valid/response.root-unsigned.assertion-signed.xml, and run tests. Tests will fail.

Expected behavior
Tests should pass.

Environment

• Node.js version: 14.15.1
• passport-saml version: 2.0.6

[markstos]

Thanks for the report.