From f1e00b64c21a725f545e675cd810bbaa435a3972 Mon Sep 17 00:00:00 2001 From: pp-ps <63303456+pp-ps@users.noreply.github.com> Date: Thu, 17 Jun 2021 17:23:37 +0200 Subject: [PATCH] Limit transforms for signed nodes (#595) * Limit transforms for signed node * Add a test to catch too many transforms in a signed node --- src/node-saml/saml.ts | 14 ++++++ test/node-saml/test-signatures.spec.ts | 17 ++++++++ ...t-signed-transforms.assertion-unsigned.xml | 43 +++++++++++++++++++ ...t-unsigned.assertion-signed-transforms.xml | 42 ++++++++++++++++++ 4 files changed, 116 insertions(+) create mode 100644 test/static/signatures/invalid/response.root-signed-transforms.assertion-unsigned.xml create mode 100644 test/static/signatures/invalid/response.root-unsigned.assertion-signed-transforms.xml diff --git a/src/node-saml/saml.ts b/src/node-saml/saml.ts index 90974d7f..e6062ebb 100644 --- a/src/node-saml/saml.ts +++ b/src/node-saml/saml.ts @@ -710,6 +710,20 @@ class SAML { 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.selectElements(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) => { diff --git a/test/node-saml/test-signatures.spec.ts b/test/node-saml/test-signatures.spec.ts index a593c82e..5cbb3172 100644 --- a/test/node-saml/test-signatures.spec.ts +++ b/test/node-saml/test-signatures.spec.ts @@ -9,6 +9,7 @@ const cert = fs.readFileSync(__dirname + "/../static/cert.pem", "ascii"); describe("Signatures", function () { const INVALID_SIGNATURE = "Invalid signature", INVALID_ENCRYPTED_SIGNATURE = "Invalid signature from encrypted assertion", + INVALID_TOO_MANY_TRANSFORMS = "Invalid signature, too many transforms", createBody = (pathToXml: string) => ({ SAMLResponse: fs.readFileSync(__dirname + "/../static/signatures" + pathToXml, "base64"), }), @@ -123,6 +124,22 @@ describe("Signatures", function () { } ) ); + it( + "R1A - root signed but with too many transforms => early error", + testOneResponse( + "/invalid/response.root-signed-transforms.assertion-unsigned.xml", + INVALID_TOO_MANY_TRANSFORMS, + 1 + ) + ); + it( + "R1A - root unsigned, asrt signed but with too many transforms => early error", + testOneResponse( + "/invalid/response.root-unsigned.assertion-signed-transforms.xml", + INVALID_TOO_MANY_TRANSFORMS, + 2 + ) + ); }); describe("Signatures on saml:Response - 1 saml:Assertion + 1 saml:Advice containing 1 saml:Assertion", () => { diff --git a/test/static/signatures/invalid/response.root-signed-transforms.assertion-unsigned.xml b/test/static/signatures/invalid/response.root-signed-transforms.assertion-unsigned.xml new file mode 100644 index 00000000..9644b189 --- /dev/null +++ b/test/static/signatures/invalid/response.root-signed-transforms.assertion-unsigned.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 + + + + + \ No newline at end of file diff --git a/test/static/signatures/invalid/response.root-unsigned.assertion-signed-transforms.xml b/test/static/signatures/invalid/response.root-unsigned.assertion-signed-transforms.xml new file mode 100644 index 00000000..c551a5fc --- /dev/null +++ b/test/static/signatures/invalid/response.root-unsigned.assertion-signed-transforms.xml @@ -0,0 +1,42 @@ + + https://evil-corp.com + + + + + https://evil-corp.com + + + a5ob8qMYkX+MR1ipeAhS6Nc/xgo=BUQlRMoZcm/8pmmouQebYcOl7l2TG26z73XyJF0QDE7gGz0nm48sw2dvKNLnp9Q5uxdNvwfzuJdHvstFZJg/bsW1C5r+9rLnIYGR8lJawpehUJLpqapUN2BAYKVPV4JkfKd3ELUgsFphCJh+1oRN0oTOBtC8Fy8wccEUto5y0AjgJjjjzhBDnWAAfj6itBedzO7HrKCodFVv0MXQti6gzxGGhcGYO6X7NKXxFkSPGjRwj1FQ2TwBuMo/hr8StXgRpr5+aWb1w62idOfNGwGjqEJWy7CiHi5UENN+/YxCYvMXqsrl7WcEG2byakFYHmQL3Ou7joSJH48AWObK/1HiVg== +MIIDtTCCAp2gAwIBAgIJAKg4VeVcIDz1MA0GCSqGSIb3DQEBBQUAMEUxCzAJBgNVBAYTAlVTMRMwEQYDVQQIEwpTb21lLVN0YXRlMSEwHwYDVQQKExhJbnRlcm5ldCBXaWRnaXRzIFB0eSBMdGQwHhcNMTUwODEzMDE1NDIwWhcNMTUwOTEyMDE1NDIwWjBFMQswCQYDVQQGEwJVUzETMBEGA1UECBMKU29tZS1TdGF0ZTEhMB8GA1UEChMYSW50ZXJuZXQgV2lkZ2l0cyBQdHkgTHRkMIIBIjANBgkqhkiG9w0BAQEFAAOCAQ8AMIIBCgKCAQEAxG3ouM7U+fXbJt69X1H6d4UNg/uRr06pFuU9RkfIwNC+yaXyptqB3ynXKsL7BFt4DCd0fflRvJAx3feJIDp16wN9GDVHcufWMYPhh2j5HcTW/j9JoIJzGhJyvO00YKBt+hHy83iN1SdChKv5y0iSyiPP5GnqFw+ayyHoM6hSO0PqBou1Xb0ZSIE+DHosBnvVna5w2AiPY4xrJl9yZHZ4Q7DfMiYTgstjETio4bX+6oLiBnYktn7DjdEslqhffVme4PuBxNojI+uCeg/sn4QVLd/iogMJfDWNuLD8326Mi/FE9cCRvFlvAiMSaebMI3zPaySsxTK7Zgj5TpEbmbHI9wIDAQABo4GnMIGkMB0GA1UdDgQWBBSVGgvoW4MhMuzBGce29PY8vSzHFzB1BgNVHSMEbjBsgBSVGgvoW4MhMuzBGce29PY8vSzHF6FJpEcwRTELMAkGA1UEBhMCVVMxEzARBgNVBAgTClNvbWUtU3RhdGUxITAfBgNVBAoTGEludGVybmV0IFdpZGdpdHMgUHR5IEx0ZIIJAKg4VeVcIDz1MAwGA1UdEwQFMAMBAf8wDQYJKoZIhvcNAQEFBQADggEBAJu1rqs+anD74dbdwgd3CnqnQsQDJiEXmBhG2leaGt3ve9b/9gKaJg2pyb2NyppDe1uLqh6nNXDuzg1oNZrPz5pJL/eCXPl7FhxhMUi04TtLf8LeNTCIWYZiFuO4pmhohHcv8kRvYR1+6SkLTC8j/TZerm7qvesSiTQFNapa1eNdVQ8nFwVkEtWl+JzKEM1BlRcn42sjJkijeFp7DpI7pU+PnYeiaXpRv5pJo8ogM1iFxN+SnfEs0EuQ7fhKIG9aHKi7bKZ7L6SyX7MDIGLeulEU6lf5D9BfXNmcMambiS0pXhL2QXajt96UBq8FT2KNXY8XNtR4y6MyyCzhaiZZcc8= + + vincent.vega@evil-corp.com + + + + + + + + + urn:oasis:names:tc:SAML:2.0:ac:classes:PasswordProtectedTransport + + + + + + + vincent.vega@evil-corp.com + + + + Vincent + + + + VEGA + + + + + \ No newline at end of file