diff --git a/lib/passport-saml/saml.js b/lib/passport-saml/saml.js index 0bd3338d..2c8a1bd0 100644 --- a/lib/passport-saml/saml.js +++ b/lib/passport-saml/saml.js @@ -596,8 +596,14 @@ 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. @@ -605,6 +611,21 @@ SAML.prototype.validateSignature = function (fullXml, currentNode, certs) { 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 index d380c421..55aed6f2 100644 --- 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 new file mode 100644 index 00000000..3bfce4ca --- /dev/null +++ b/test/static/response.too-many-transforms.xml @@ -0,0 +1,43 @@ + + + https://evil-corp.com + + + 5669VeX2/9m5/1BEojrL+YgFlMI=aLZVEDEna5792s0Kn1UtS++N7EUs30jtHoTa4DFVRvVnPUr7xw77SmDr+HHSupVTh7BdA3T+gW5+pwbGqGrG6+CEQEYxF8arIHUlrx6N+nvPpIpJrsEOcfpj0xxBLj8d0Yh5zXIq5JEX9lcZ/JVOCLzK0Rn024OpARxo992K/wqKcXOEnFJP6xGsSaAed3qQu/5+lSbLeS9i9bJJv9G0ab8zZMR+9CmPV0PQxDZIw5f0CNjvmsZ0qPHXSL5fbdfhDHNd3VRJkiLyA9YpG5s7izAiqiFwsXR2x2kn4RrZfv6sajZltDVl9+ejpkHn9ZOV+SfAe2p7LyHKxJFMTlwbFQ== +MIIDtTCCAp2gAwIBAgIJAKg4VeVcIDz1MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMTUwODEzMDE1NDIwWhcNMTUwOTEyMDE1NDIwWjBFMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxG3ouM7U+fXbJt69X1H6d4UNg/uRr06pFuU9RkfIwNC+yaXyptqB3ynXKsL7BFt4DCd0fflRvJAx3feJIDp16wN9GDVHcufWMYPhh2j5HcTW/j9JoIJzGhJyvO00YKBt+hHy83iN1SdChKv5y0iSyiPP5GnqFw+ayyHoM6hSO0PqBou1Xb0ZSIE+DHosBnvVna5w2AiPY4xrJl9yZHZ4Q7DfMiYTgstjETio4bX+6oLiBnYktn7DjdEslqhffVme4PuBxNojI+uCeg/sn4QVLd/iogMJfDWNuLD8326Mi/FE9cCRvFlvAiMSaebMI3zPaySsxTK7Zgj5TpEbmbHI9wIDAQABo4GnMIGkMB0GA1UdDgQWBBSVGgvoW4MhMuzBGce29PY8vSzHFzB1BgNVHSMEbjBsgBSVGgvoW4MhMuzBGce29PY8vSzHF6FJpEcwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAKg4VeVcIDz1MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAJu1rqs+anD74dbdwgd3CnqnQsQDJiEXmBhG2leaGt3ve9b/9gKaJg2pyb2NyppDe1uLqh6nNXDuzg1oNZrPz5pJL/eCXPl7FhxhMUi04TtLf8LeNTCIWYZiFuO4pmhohHcv8kRvYR1+6SkLTC8j/TZerm7qvesSiTQFNapa1eNdVQ8nFwVkEtWl+JzKEM1BlRcn42sjJkijeFp7DpI7pU+PnYeiaXpRv5pJo8ogM1iFxN+SnfEs0EuQ7fhKIG9aHKi7bKZ7L6SyX7MDIGLeulEU6lf5D9BfXNmcMambiS0pXhL2QXajt96UBq8FT2KNXY8XNtR4y6MyyCzhaiZZcc8= + + + + + https://evil-corp.com + + vincent.vega@evil-corp.com + + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + vincent.vega@evil-corp.com + + + + Vincent + + + + VEGA + + + + + diff --git a/test/tests.js b/test/tests.js index ce50cacb..3eda17ca 100644 --- 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 = {