Merge pull request #30 from vrk-kpa/add-security-patch-to-mitigate-un…

…limited-transforms Added security patch for too many transforms 'GHSA-5379-r78w-42h2'
vrk-kpa · Oct 17, 2022 · 4dd46bc · 4dd46bc
2 parents 10d947b + 990a166
commit 4dd46bc
Show file tree

Hide file tree

Showing 4 changed files with 89 additions and 3 deletions.
diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js
@@ -596,15 +596,36 @@ SAML.prototype.certsToCheck = function () {
 // See https://github.com/bergie/passport-saml/issues/19 for references to some of the attack
 //   vectors against SAML signature verification.
 SAML.prototype.validateSignature = function (fullXml, currentNode, certs) {
-  const xpathSigQuery = ".//*[local-name(.)='Signature' and " +
-                      "namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
+  const xpathSigQuery =
+      ".//*[" +
+      "local-name(.)='Signature' and " +
+      "namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#' and " +
+      "descendant::*[local-name(.)='Reference' and @URI='#" +
+      currentNode.getAttribute("ID") +
+      "']" +
+      "]";
   const signatures = xpath(currentNode, xpathSigQuery);
   // This function is expecting to validate exactly one signature, so if we find more or fewer
   //   than that, reject.
   if (signatures.length != 1) {
     return false;
   }
 
+  const xpathTransformQuery =
+      ".//*[" +
+      "local-name(.)='Transform' and " +
+      "namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#' and " +
+      "ancestor::*[local-name(.)='Reference' and @URI='#" +
+      currentNode.getAttribute("ID") +
+      "']" +
+      "]";
+  const transforms = xpath(currentNode, xpathTransformQuery);
+  // Reject also XMLDSIG with more than 2 Transform
+  if (transforms.length > 2) {
+    // do not return false, throw an error so that it can be caught by tests differently
+    throw new Error("Invalid signature, too many transforms");
+  }
+
   const signature = signatures[0];
   return certs.some(certToCheck => {
     return this.validateSignatureForCert(signature, certToCheck, fullXml, currentNode);

diff --git a/package.json b/package.json
@@ -1,6 +1,6 @@
 {
   "name": "suomifi-passport-saml",
-  "version": "1.3.3-sfi.9",
+  "version": "1.3.3-sfi.10",
   "license": "MIT",
   "keywords": [
     "saml",

diff --git a/test/static/response.too-many-transforms.xml b/test/static/response.too-many-transforms.xml
@@ -0,0 +1,43 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" Destination="https://evil-corp.madness.com/sso/callback" ID="pfxea164cc1-96ac-af95-85e8-058c9d279cc5" InResponseTo="_e8df3fe5f04237d25670" IssueInstant="2015-08-31T08:54:06+00:00" Version="2.0">
+    <saml:Issuer>https://evil-corp.com</saml:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
+  <ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
+    <ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
+  <ds:Reference URI="#pfxea164cc1-96ac-af95-85e8-058c9d279cc5"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>5669VeX2/9m5/1BEojrL+YgFlMI=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>aLZVEDEna5792s0Kn1UtS++N7EUs30jtHoTa4DFVRvVnPUr7xw77SmDr+HHSupVTh7BdA3T+gW5+pwbGqGrG6+CEQEYxF8arIHUlrx6N+nvPpIpJrsEOcfpj0xxBLj8d0Yh5zXIq5JEX9lcZ/JVOCLzK0Rn024OpARxo992K/wqKcXOEnFJP6xGsSaAed3qQu/5+lSbLeS9i9bJJv9G0ab8zZMR+9CmPV0PQxDZIw5f0CNjvmsZ0qPHXSL5fbdfhDHNd3VRJkiLyA9YpG5s7izAiqiFwsXR2x2kn4RrZfv6sajZltDVl9+ejpkHn9ZOV+SfAe2p7LyHKxJFMTlwbFQ==</ds:SignatureValue>
+<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIIDtTCCAp2gAwIBAgIJAKg4VeVcIDz1MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMTUwODEzMDE1NDIwWhcNMTUwOTEyMDE1NDIwWjBFMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxG3ouM7U+fXbJt69X1H6d4UNg/uRr06pFuU9RkfIwNC+yaXyptqB3ynXKsL7BFt4DCd0fflRvJAx3feJIDp16wN9GDVHcufWMYPhh2j5HcTW/j9JoIJzGhJyvO00YKBt+hHy83iN1SdChKv5y0iSyiPP5GnqFw+ayyHoM6hSO0PqBou1Xb0ZSIE+DHosBnvVna5w2AiPY4xrJl9yZHZ4Q7DfMiYTgstjETio4bX+6oLiBnYktn7DjdEslqhffVme4PuBxNojI+uCeg/sn4QVLd/iogMJfDWNuLD8326Mi/FE9cCRvFlvAiMSaebMI3zPaySsxTK7Zgj5TpEbmbHI9wIDAQABo4GnMIGkMB0GA1UdDgQWBBSVGgvoW4MhMuzBGce29PY8vSzHFzB1BgNVHSMEbjBsgBSVGgvoW4MhMuzBGce29PY8vSzHF6FJpEcwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAKg4VeVcIDz1MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAJu1rqs+anD74dbdwgd3CnqnQsQDJiEXmBhG2leaGt3ve9b/9gKaJg2pyb2NyppDe1uLqh6nNXDuzg1oNZrPz5pJL/eCXPl7FhxhMUi04TtLf8LeNTCIWYZiFuO4pmhohHcv8kRvYR1+6SkLTC8j/TZerm7qvesSiTQFNapa1eNdVQ8nFwVkEtWl+JzKEM1BlRcn42sjJkijeFp7DpI7pU+PnYeiaXpRv5pJo8ogM1iFxN+SnfEs0EuQ7fhKIG9aHKi7bKZ7L6SyX7MDIGLeulEU6lf5D9BfXNmcMambiS0pXhL2QXajt96UBq8FT2KNXY8XNtR4y6MyyCzhaiZZcc8=</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature>
+    <samlp:Status>
+        <samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/>
+    </samlp:Status>
+    <saml:Assertion ID="_bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb" IssueInstant="2020-09-25T16:00:00+00:00" Version="2.0">
+        <saml:Issuer>https://evil-corp.com</saml:Issuer>
+        <saml:Subject>
+            <saml:NameID Format="urn:oasis:names:tc:SAML:1.1:nameid-format:emailAddress">vincent.vega@evil-corp.com
+            </saml:NameID>
+            <saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer">
+                <saml:SubjectConfirmationData InResponseTo="_e8df3fe5f04237d25670" NotOnOrAfter="2020-09-25T17:00:00+00:00" Recipient="https://evil-corp.madness.com/sso/callback"/>
+            </saml:SubjectConfirmation>
+        </saml:Subject>
+        <saml:Conditions NotBefore="2020-09-25T16:00:00+00:00" NotOnOrAfter="2020-09-25T17:00:00+00:00"/>
+        <saml:AuthnStatement AuthnInstant="2020-09-25T16:00:00+00:00" SessionIndex="_9e315bdf7b1b6732be33c377cf6f5c4f">
+            <saml:AuthnContext>
+                <saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport
+                </saml:AuthnContextClassRef>
+            </saml:AuthnContext>
+        </saml:AuthnStatement>
+        <saml:AttributeStatement>
+            <saml:Attribute Name="evil-corp.egroupid">
+                <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">
+                    vincent.vega@evil-corp.com
+                </saml:AttributeValue>
+            </saml:Attribute>
+            <saml:Attribute Name="evilcorp.givenname">
+                <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">Vincent
+                </saml:AttributeValue>
+            </saml:Attribute>
+            <saml:Attribute Name="evilcorp.sn">
+                <saml:AttributeValue xmlns:xs="http://www.w3.org/2001/XMLSchema" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:type="xs:string">VEGA
+                </saml:AttributeValue>
+            </saml:Attribute>
+        </saml:AttributeStatement>
+    </saml:Assertion>
+</samlp:Response>
diff --git a/test/tests.js b/test/tests.js
@@ -1107,6 +1107,28 @@ describe( 'passport-saml /', function() {
         });
       });
 
+      it('response with too many transforms', function(done) {
+        var fakeClock = sinon.useFakeTimers(Date.parse('2020-09-25T16:10:00+00:00'));
+        var container = {
+            SAMLResponse : fs.readFileSync(
+                __dirname + '/static/response.too-many-transforms.xml'
+            ).toString('base64')
+        };
+        var samlObj = new SAML( { cert: TEST_CERT, suomifiAdditions: suomifiAdditionsOptions } );
+
+        samlObj.validatePostResponse(container, function(err, profile) {
+          try {
+            should.exist(err);
+            err.message.should.eql('Invalid signature, too many transforms');
+            fakeClock.restore();
+
+            done();
+          } catch (err2) {
+            done(err2);
+          }
+        });
+      });
+
       it('accept response with an attributeStatement element without attributeValue', function(done) {
         var fakeClock = sinon.useFakeTimers(Date.parse('2015-08-31T08:55:00+00:00'));
         var container = {