CVE-2025-54419

@@ -62,7 +62,7 @@
     "@xmldom/is-dom-node": "^1.0.1",
     "@xmldom/xmldom": "^0.8.10",
     "debug": "^4.4.0",
-    "xml-crypto": "^6.1.0",
+    "xml-crypto": "^6.1.2",
     "xml-encryption": "^3.1.0",
     "xml2js": "^0.6.2",
     "xmlbuilder": "^15.1.1",

@@ -16,7 +16,7 @@
         "@xmldom/is-dom-node": "^1.0.1",
         "@xmldom/xmldom": "^0.8.10",
         "debug": "^4.4.0",
-        "xml-crypto": "^6.1.0",
+        "xml-crypto": "^6.1.2",
         "xml-encryption": "^3.1.0",
         "xml2js": "^0.6.2",
         "xmlbuilder": "^15.1.1",
@@ -8558,9 +8558,9 @@
       }
     },
     "node_modules/xml-crypto": {
-      "version": "6.1.0",
-      "resolved": "https://registry.npmjs.org/xml-crypto/-/xml-crypto-6.1.0.tgz",
-      "integrity": "sha512-0TYPBRPwXLnRGc2F0f9Zc/H076YcP7tkCa2US4jpguuPTEx7TWFqSysIfJ1hP4r2KF82IYzhnzepnsUEsOjlOw==",
+      "version": "6.1.2",
+      "resolved": "https://registry.npmjs.org/xml-crypto/-/xml-crypto-6.1.2.tgz",
+      "integrity": "sha512-leBOVQdVi8FvPJrMYoum7Ici9qyxfE4kVi+AkpUoYCSXaQF4IlBm1cneTK9oAxR61LpYxTx7lNcsnBIeRpGW2w==",
       "license": "MIT",
       "dependencies": {
         "@xmldom/is-dom-node": "^1.0.1",

@@ -34,6 +34,7 @@ import {
   buildXmlBuilderObject,
   decryptXml,
   getNameIdAsync,
+  getVerifiedXml,
   parseDomFromString,
   parseXml2JsFromString,
   validateSignature,
@@ -282,9 +283,7 @@ class SAML {
     }
 
     if (this.options.scoping != null) {
-      const scoping: XMLInput = {
-        "@xmlns:samlp": "urn:oasis:names:tc:SAML:2.0:protocol",
-      };
+      const scoping: XMLInput = { "@xmlns:samlp": "urn:oasis:names:tc:SAML:2.0:protocol" };
 
       if (typeof this.options.scoping.proxyCount === "number") {
         scoping["@ProxyCount"] = this.options.scoping.proxyCount;
@@ -360,10 +359,7 @@ class SAML {
           "#text": this.options.issuer,
         },
         "samlp:Extensions": {},
-        "saml:NameID": {
-          "@Format": user.nameIDFormat,
-          "#text": user.nameID,
-        },
+        "saml:NameID": { "@Format": user.nameIDFormat, "#text": user.nameID },
       },
     } as LogoutRequestXML;
 
@@ -404,17 +400,13 @@ class SAML {
     const instant = generateInstant();
 
     const successStatus = {
-      "samlp:StatusCode": {
-        "@Value": "urn:oasis:names:tc:SAML:2.0:status:Success",
-      },
+      "samlp:StatusCode": { "@Value": "urn:oasis:names:tc:SAML:2.0:status:Success" },
     };
 
     const failStatus = {
       "samlp:StatusCode": {
         "@Value": "urn:oasis:names:tc:SAML:2.0:status:Requester",
-        "samlp:StatusCode": {
-          "@Value": "urn:oasis:names:tc:SAML:2.0:status:UnknownPrincipal",
-        },
+        "samlp:StatusCode": { "@Value": "urn:oasis:names:tc:SAML:2.0:status:UnknownPrincipal" },
       },
     };
 
@@ -427,9 +419,7 @@ class SAML {
         "@IssueInstant": instant,
         "@Destination": this.options.logoutUrl,
         "@InResponseTo": logoutRequest.ID,
-        "saml:Issuer": {
-          "#text": this.options.issuer,
-        },
+        "saml:Issuer": { "#text": this.options.issuer },
         "samlp:Status": success ? successStatus : failStatus,
       },
     };
@@ -466,12 +456,8 @@ class SAML {
     }
 
     const samlMessage: querystring.ParsedUrlQuery = request
-      ? {
-          SAMLRequest: base64,
-        }
-      : {
-          SAMLResponse: base64,
-        };
+      ? { SAMLRequest: base64 }
+      : { SAMLResponse: base64 };
     Object.keys(additionalParameters).forEach((k) => {
       samlMessage[k] = additionalParameters[k];
     });
@@ -545,9 +531,7 @@ class SAML {
     const operation = "authorize";
     const overrideParams = options ? options.additionalParams || {} : {};
     const additionalParameters = this._getAdditionalParams(RelayState, operation, overrideParams);
-    const samlMessage: querystring.ParsedUrlQueryInput = {
-      SAMLRequest: buffer.toString("base64"),
-    };
+    const samlMessage: querystring.ParsedUrlQueryInput = { SAMLRequest: buffer.toString("base64") };
 
     Object.keys(additionalParameters).forEach((k) => {
       samlMessage[k] = additionalParameters[k] || "";
@@ -570,12 +554,11 @@ class SAML {
       s:
         | string
         | number
+        | bigint
         | boolean
         | undefined
         | null
-        | readonly string[]
-        | readonly number[]
-        | readonly boolean[],
+        | readonly (string | number | bigint | boolean)[],
       preserveCR?: boolean,
     ) {
       const preserveCRChar = preserveCR ? "
" : "\n";
@@ -681,6 +664,48 @@ class SAML {
     return this.pemFiles;
   }
 
+  // given actually signed XML, try to get the actual assertion used
+  protected async getSignedAssertion(signedXml: string): Promise<string | null> {
+    // case 1: Response signed
+    const verifiedDoc = await parseDomFromString(signedXml);
+    const rootNode = verifiedDoc.documentElement;
+
+    // case 1: response is a verified assertion
+    if (rootNode.localName === "Response") {
+      // try getting the Xml from the assertions
+      const assertions = xpath.selectElements(rootNode, "./*[local-name()='Assertion']");
+      // now we can process the assertion as an assertion
+      if (assertions.length == 1) {
+        return assertions[0].toString();
+      }
+      // encrypted assertion
+      const encryptedAssertions = xpath.selectElements(
+        rootNode,
+        "./*[local-name()='EncryptedAssertion']",
+      );
+
+      if (encryptedAssertions.length === 1) {
+        assertRequired(this.options.decryptionPvk, "No decryption key for encrypted SAML response");
+
+        const encryptedAssertionXml = encryptedAssertions[0].toString();
+
+        const decryptedXml = await decryptXml(encryptedAssertionXml, this.options.decryptionPvk);
+        const decryptedDoc = await parseDomFromString(decryptedXml);
+        const decryptedAssertion = decryptedDoc.documentElement;
+        if (decryptedAssertion.localName !== "Assertion") {
+          throw new Error("Invalid EncryptedAssertion content");
+        }
+
+        return decryptedAssertion.toString();
+      }
+    } else if (rootNode.localName === "Assertion") {
+      return rootNode.toString();
+    } else {
+      return null;
+    }
+    return null;
+  }
+
   async validatePostResponseAsync(
     container: Record<string, string>,
   ): Promise<{ profile: Profile | null; loggedOut: boolean }> {
@@ -704,8 +729,13 @@ class SAML {
       }
       const pemFiles = await this.getKeyInfosAsPem();
       // Check if this document has a valid top-level signature which applies to the entire XML document
-      let validSignature = false;
-      if (validateSignature(xml, doc.documentElement, pemFiles)) {
+      let validSignature = false; // Use `getVerifiedXml()` to collect the actual verified contents
+
+      const responseVerifiedXml = getVerifiedXml(xml, doc.documentElement, pemFiles);
+      let assertionVerifiedXml = null;
+      let decryptedAssertionVerifiedXml = null;
+
+      if (responseVerifiedXml) {
         validSignature = true;
       }
 
@@ -729,18 +759,12 @@ class SAML {
       }
 
       if (assertions.length == 1) {
-        if (
-          (this.options.wantAssertionsSigned || !validSignature) &&
-          !validateSignature(xml, assertions[0], pemFiles)
-        ) {
-          throw new Error("Invalid signature");
+        if (this.options.wantAssertionsSigned || !validSignature) {
+          assertionVerifiedXml = getVerifiedXml(xml, assertions[0], pemFiles);
+          if (!assertionVerifiedXml) {
+            throw new Error("Invalid signature");
+          }
         }
-
-        return await this.processValidlySignedAssertionAsync(
-          assertions[0].toString(),
-          xml,
-          inResponseTo,
-        );
       }
 
       if (encryptedAssertions.length == 1) {
@@ -756,22 +780,33 @@ class SAML {
         );
         if (decryptedAssertions.length != 1) throw new Error("Invalid EncryptedAssertion content");
 
-        if (
-          (this.options.wantAssertionsSigned || !validSignature) &&
-          !validateSignature(decryptedXml, decryptedAssertions[0], pemFiles)
-        ) {
-          throw new Error("Invalid signature from encrypted assertion");
+        if (this.options.wantAssertionsSigned || !validSignature) {
+          decryptedAssertionVerifiedXml = getVerifiedXml(
+            decryptedXml,
+            decryptedAssertions[0],
+            pemFiles,
+          );
+          if (decryptedAssertionVerifiedXml == null) {
+            throw new Error("Invalid signature from encrypted assertion");
+          }
         }
-
-        return await this.processValidlySignedAssertionAsync(
-          decryptedAssertions[0].toString(),
-          xml,
-          inResponseTo,
-        );
       }
 
       // If there's no assertion, fall back on xml2js response parsing for the status &
       //   LogoutResponse code.
+      // collect the verified XML's
+      const verifiedXml =
+        responseVerifiedXml || assertionVerifiedXml || decryptedAssertionVerifiedXml;
+
+      // double check that there is at least 1 assertion
+      if (verifiedXml && assertions.length + encryptedAssertions.length == 1) {
+        const signedAssertion = await this.getSignedAssertion(verifiedXml);
+
+        if (signedAssertion == null) {
+          throw new Error("Cannot obtain assertion from signed data");
+        }
+        return await this.processValidlySignedAssertionAsync(signedAssertion, xml, inResponseTo);
+      }
 
       const xmljsDoc = (await parseXml2JsFromString(xml)) as SamlResponseXmlJs;
       const response = xmljsDoc.Response;
@@ -981,8 +1016,8 @@ class SAML {
 
   protected async processValidlySignedAssertionAsync(
     this: SAML,
-    xml: string,
-    samlResponseXml: string,
+    xml: string, // assertion XML
+    samlResponseXml: string, // should be deprecated, this is unsigned
     inResponseTo: string | null,
   ): Promise<{ profile: Profile; loggedOut: boolean }> {
     let msg;
@@ -1323,11 +1358,7 @@ class SAML {
     decryptionCert: string | null,
     publicCerts?: string | string[] | null,
   ): string {
-    return generateServiceProviderMetadata({
-      ...this.options,
-      decryptionCert,
-      publicCerts,
-    });
+    return generateServiceProviderMetadata({ ...this.options, decryptionCert, publicCerts });
   }
 
   /**

@@ -58,25 +58,119 @@ const normalizeNewlines = (xml: string): string => {
 };
 
 /**
+ * // modeled after the current validateSignature method, to maintain consistency for unit tests
+ * Input: fullXml, the document for SignedXML context
+ * Input: currentNode, this node must have a Signature
+ * Input: pemFiles: a list of pem encoded certificates that are trusted. User is responsible for ensuring trust
+ * Find's a signature for the currentNode
+ * Return the verified contents if verified?
+ * Otherwise returns null
+ * */
+export const getVerifiedXml = (
+  fullXml: string,
+  currentNode: Element,
+  pemFiles: string[],
+): string | null => {
+  fullXml = normalizeNewlines(fullXml);
+
+  // find any signature
+  const signatures = xpath.selectElements(
+    currentNode,
+    "./*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
+  );
+  if (signatures.length < 1) {
+    return null;
+  }
+
+  if (signatures.length > 1) {
+    throw new Error("Too many signatures found for this element");
+  }
+
+  const signature = signatures[0];
+
+  const xpathTransformQuery =
+    ".//*[local-name(.)='Transform' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']";
+  const transforms = xpath.selectElements(signature, 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");
+  }
+
+  for (const pemFile of pemFiles) {
+    const sig = new xmlCrypto.SignedXml();
+    sig.publicCert = pemFile; // public certificate to verify
+    sig.loadSignature(signature);
+
+    // here are the sanity checks
+    // They do not affect the actual security of the program
+    // more so to check conformance with the SAML spec
+    const refs = sig.getReferences();
+
+    if (refs.length !== 1) return null;
+    if (!signature.parentNode) {
+      return null;
+    }
+
+    const ref = refs[0];
+
+    // only allow enveloped signature
+    const refUri = ref.uri;
+
+    const refId = refUri[0] === "#" ? refUri.substring(1) : refUri;
+
+    assertRequired(refId, "signature reference uri not found");
+    // prevent XPath injection
+    if (refId.includes("'") || refId.includes('"')) {
+      throw new Error("ref URI included quote character ' or \". Not a valid ID, and not allowed");
+    }
+
+    const totalReferencedNodes = xpath.selectElements(
+      signature.ownerDocument,
+      `//*[@ID="${refId}"]`,
+    );
+
+    if (totalReferencedNodes.length !== 1) {
+      throw new Error("Invalid signature: ID cannot refer to more than one element");
+    }
+
+    if (totalReferencedNodes[0] !== signature.parentNode) {
+      throw new Error("Invalid signature: Referenced node does not refer to it's parent element");
+    }
+
+    // actual cryptographic verification
+    // after verification, the referenced XML will be in `sig.signedReferences`
+    // do not trust any other xml (including referencedNode)
+
+    try {
+      if (!sig.checkSignature(fullXml)) {
+        continue; // no signatures verified
+      }
+
+      if (sig.getSignedReferences().length !== 1) {
+        throw new Error("Only 1 signed references should be present in signature");
+      }
+
+      return sig.getSignedReferences()[0];
+    } catch {
+      // return null; // we don't return null, since we have to verify with another key
+    }
+  }
+
+  return null;
+};
+
+/**
+ * Internally deprecated Do not only return boolean value, instead return the actual signed content. SAML Libraries must only use the referenced bytes from the signature
  * This function checks that the |currentNode| in the |fullXml| document contains exactly 1 valid
  *   signature of the |currentNode|.
  *
  * See https://github.com/bergie/passport-saml/issues/19 for references to some of the attack
  *   vectors against SAML signature verification.
  */
-export const validateSignature = (
-  fullXml: string,
-  currentNode: Element,
-  pemFiles: string[],
-): boolean => {
-  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 _validateSignature = (fullXml: string, currentNode: Element, pemFiles: string[]): boolean => {
+  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.selectElements(currentNode, xpathSigQuery);
   // This function is expecting to validate exactly one signature, so if we find more or fewer
   //   than that, reject.
@@ -104,8 +198,15 @@ export const validateSignature = (
   });
 };
 
+// validateSignature is deprecated, should be using getVerifiedXml
+// Existing non-sensitive callers can still use validateSignature
+// but new callers should use getVerifiedXml
+// this allows us to deprecate it without raising a warning
+export const validateSignature = _validateSignature;
+
 /**
  * This function checks that the |signature| is signed with a given |pemFile|.
+ * Internally deprecated, users should not be using this for anything new
  */
 const validateXmlSignatureWithPemFile = (
   signature: Node,
@@ -121,7 +222,6 @@ const validateXmlSignatureWithPemFile = (
   if (sig.getReferences().length !== 1) return false;
   const t = sig.getReferences();
   const refUri = t[0].uri;
-  // const refUri = sig.references[0].uri;
   assertRequired(refUri, "signature reference uri not found");
   const refId = refUri[0] === "#" ? refUri.substring(1) : refUri;
   // If we can't find the reference at the top level, reject
@@ -176,9 +276,7 @@ export const signXml = (
   sig.privateKey = options.privateKey;
   sig.publicCert = options.publicCert;
   sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#";
-  sig.computeSignature(xml, {
-    location,
-  });
+  sig.computeSignature(xml, { location });
 
   return sig.getSignedXml();
 };
@@ -198,10 +296,7 @@ export const parseDomFromString = (xml: string): Promise<Document> => {
        * you can override the errorHandler for xml parser
        * @link http://www.saxproject.org/apidoc/org/xml/sax/ErrorHandler.html
        */
-      errorHandler: {
-        error: errHandler,
-        fatalError: errHandler,
-      },
+      errorHandler: { error: errHandler, fatalError: errHandler },
     }).parseFromString(xml, "text/xml");
 
     if (!Object.prototype.hasOwnProperty.call(dom, "documentElement")) {
@@ -223,10 +318,7 @@ export const parseXml2JsFromString = async (xml: string | Buffer): Promise<XmlJs
 };
 
 export const buildXml2JsObject = (rootName: string, xml: XmlJsObject): string => {
-  const builderOpts = {
-    rootName,
-    headless: true,
-  };
+  const builderOpts = { rootName, headless: true };
   return new xml2js.Builder(builderOpts).buildObject(xml);
 };
 
@@ -237,10 +329,7 @@ export const buildXmlBuilderObject = (xml: XMLOutput, pretty: boolean): string =
 
 export const promiseWithNameId = async (nameid: Node): Promise<NameID> => {
   const format = xpath.selectAttributes(nameid, "@Format");
-  return {
-    value: nameid.textContent,
-    format: format && format[0] && format[0].nodeValue,
-  };
+  return { value: nameid.textContent, format: format && format[0] && format[0].nodeValue };
 };
 
 export const getNameIdAsync = async (

@@ -23,7 +23,7 @@ describe("Signatures", function () {
   let validateSignatureSpy: sinon.SinonSpy;
 
   beforeEach(() => {
-    validateSignatureSpy = sinon.spy(xml, "validateSignature");
+    validateSignatureSpy = sinon.spy(xml, "getVerifiedXml");
   });
 
   afterEach(() => {

@@ -11,7 +11,7 @@ import { expect } from "chai";
 import * as assert from "assert";
 import { FAKE_CERT, TEST_CERT } from "./types";
 import { assertRequired, signXmlResponse } from "../src/utility";
-import { parseDomFromString, validateSignature } from "../src/xml";
+import { getVerifiedXml, parseDomFromString, validateSignature } from "../src/xml";
 import { generateServiceProviderMetadata } from "../src/metadata";
 
 const BAD_TEST_CERT =
@@ -678,6 +678,7 @@ describe("node-saml /", function () {
 
         const dom = await parseDomFromString(metadata);
         expect(validateSignature(metadata, dom.documentElement, [publicCert])).to.be.true;
+        assert(getVerifiedXml(metadata, dom.documentElement, [publicCert]));
       });
 
       it("generateServiceProviderMetadata contains metadataExtensions", function () {