node-saml · ahacker1-securesaml · Sep 8, 2025 · Sep 11, 2025
diff --git a/package-lock.json b/package-lock.json
diff --git a/package.json b/package.json
@@ -61,7 +61,6 @@
     "@types/xml2js": "^0.4.14",
     "@xmldom/is-dom-node": "^1.0.1",
     "@xmldom/xmldom": "^0.8.10",
-    "debug": "^4.4.0",
     "xml-crypto": "^6.1.2",
     "xml-encryption": "^3.1.0",
     "xml2js": "^0.6.2",

diff --git a/src/saml.ts b/src/saml.ts
@@ -1,4 +1,3 @@
-import Debug from "debug";
 import * as zlib from "zlib";
 import * as crypto from "crypto";
 import { URL } from "url";
@@ -37,7 +36,6 @@ import {
   getVerifiedXml,
   parseDomFromString,
   parseXml2JsFromString,
-  validateSignature,
   xpath,
 } from "./xml";
 import { keyInfoToPem, generateUniqueId } from "./crypto";
@@ -46,7 +44,6 @@ import { signAuthnRequestPost } from "./saml-post-signing";
 import { generateServiceProviderMetadata } from "./metadata";
 import { DEFAULT_IDENTIFIER_FORMAT, DEFAULT_WANT_ASSERTIONS_SIGNED } from "./constants";
 
-const debug = Debug("node-saml");
 const inflateRawAsync = util.promisify(zlib.inflateRaw);
 const deflateRawAsync = util.promisify(zlib.deflateRaw);
 
@@ -866,7 +863,7 @@ class SAML {
         }
       }
     } catch (err) {
-      debug("validatePostResponse resulted in an error: %s", err);
+      // debug("validatePostResponse resulted in an error: %s", err);
       if (this.mustValidateInResponseTo(Boolean(inResponseTo))) {
         await this.cacheProvider.removeAsync(inResponseTo);
       }
@@ -1284,17 +1281,20 @@ class SAML {
     {
       _parseDomFromString = parseDomFromString,
       _parseXml2JsFromString = parseXml2JsFromString,
-      _validateSignature = validateSignature,
+      _getVerifiedXml = getVerifiedXml,
     } = {},
   ): Promise<{ profile: Profile; loggedOut: boolean }> {
     const xml = Buffer.from(container.SAMLRequest, "base64").toString("utf8");
     const dom = await _parseDomFromString(xml);
-    const doc = await _parseXml2JsFromString(xml);
     const pemFiles = await this.getKeyInfosAsPem();
-    if (!_validateSignature(xml, dom.documentElement, pemFiles)) {
+    const verifiedXml = _getVerifiedXml(xml, dom.documentElement, pemFiles);
+
+    if (!verifiedXml) {
       throw new Error("Invalid signature on documentElement");
     }
-    return await this.processValidlySignedPostRequestAsync(doc, dom);
+    const verifiedRequest = await _parseXml2JsFromString(verifiedXml);
+
+    return await this.processValidlySignedPostRequestAsync(verifiedRequest, dom);
   }
 
   protected async processValidlySignedPostRequestAsync(

diff --git a/src/xml.ts b/src/xml.ts
@@ -16,9 +16,7 @@ import {
 import * as algorithms from "./algorithms";
 import { assertRequired } from "./utility";
 import * as isDomNode from "@xmldom/is-dom-node";
-import Debug from "debug";
 
-const debug = Debug("node-saml");
 
 const selectXPath = <T extends Node>(
   guard: (values: SelectReturnType) => values is Array<T>,
@@ -160,49 +158,6 @@ export const getVerifiedXml = (
   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.
- */
-
-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.
-  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 pemFiles.some((pemFile) => {
-    return validateXmlSignatureWithPemFile(signature, pemFile, fullXml, currentNode);
-  });
-};
-
-// 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|.
@@ -242,7 +197,7 @@ const validateXmlSignatureWithPemFile = (
   try {
     return sig.checkSignature(fullXml);
   } catch (err) {
-    debug("signature check resulted in an error: %s", err);
+    //debug("signature check resulted in an error: %s", err);
     return false;
   }
 };

diff --git a/test/samlTests.spec.ts b/test/samlTests.spec.ts
@@ -8,7 +8,7 @@ import { SAML } from "../src/saml";
 import { AuthOptions, IdpCertCallback } from "../src/types";
 import { assertRequired } from "../src/utility";
 import { FAKE_CERT, RequestWithUser, TEST_CERT_MULTILINE } from "./types";
-import { parseDomFromString, parseXml2JsFromString, validateSignature } from "../src/xml";
+import { getVerifiedXml, parseDomFromString, parseXml2JsFromString } from "../src/xml";
 
 const noop = (): void => undefined;
 
@@ -56,8 +56,8 @@ describe("saml.ts", function () {
             return { documentElement: null };
           }) as unknown as typeof parseDomFromString,
           _parseXml2JsFromString: noop as unknown as typeof parseXml2JsFromString,
-          _validateSignature: (() => true) as unknown as typeof validateSignature,
-        },
+          _getVerifiedXml: ((fullXml: string, currentNode: Element, pemFiles: string[]) => fullXml) as unknown as typeof getVerifiedXml,
+        }
       );
 
       const pendingResult = getKeyInfosAsPemSpy.returnValues[0];

diff --git a/test/tests.spec.ts b/test/tests.spec.ts
@@ -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 { getVerifiedXml, parseDomFromString, validateSignature } from "../src/xml";
+import { getVerifiedXml, parseDomFromString } from "../src/xml";
 import { generateServiceProviderMetadata } from "../src/metadata";
 
 const BAD_TEST_CERT =
@@ -677,7 +677,6 @@ describe("node-saml /", function () {
         const metadata = samlObj.generateServiceProviderMetadata(null, publicCert);
 
         const dom = await parseDomFromString(metadata);
-        expect(validateSignature(metadata, dom.documentElement, [publicCert])).to.be.true;
         assert(getVerifiedXml(metadata, dom.documentElement, [publicCert]));
       });