CVE-2024-32962: Insufficient Information (NVD-CWE-noinfo)
value: xml-crypto is an xml digital signature and encryption library for Node.js. In affected versions the default configuration does not check authorization of the signer, it only checks the validity of the signature per section 3.2.2 of the w3 xmldsig-core-20080610 spec. As such, without additional validation steps, the default configuration allows a malicious actor to re-sign an XML document, place the certificate in a `<KeyInfo />` element, and pass `xml-crypto` default validation checks. As a result `xml-crypto` trusts by default any certificate provided via digitally signed XML document's `<KeyInfo />`. `xml-crypto` prefers to use any certificate provided via digitally signed XML document's `<KeyInfo />` even if library was configured to use specific certificate (`publicCert`) for signature verification purposes.  An attacker can spoof signature verification by modifying XML document and replacing existing signature with signature generated with malicious private key (created by attacker) and by attaching that private key's certificate to `<KeyInfo />` element. This vulnerability is combination of changes introduced to `4.0.0` on pull request 301 / commit `c2b83f98` and has been addressed in version 6.0.0 with pull request 445 / commit `21201723d`. Users are advised to upgrade. Users unable to upgrade may either check the certificate extracted via `getCertFromKeyInfo` against trusted certificates before accepting the results of the validation or set `xml-crypto's getCertFromKeyInfo` to `() => undefined` forcing `xml-crypto` to use an explicitly configured `publicCert` or `privateKey` for signature verification.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-05-02T07:15Z
Repository: https://github.com/node-saml/xml-crypto
Language: TypeScript
Files changed: src/signed-xml.ts, test/document-tests.spec.ts, test/key-info-tests.spec.ts, test/signature-unit-tests.spec.ts
Commit date: 2024-01-26 13:11:52 -0500
Commit message: Set `getCertFromKeyInfo` to `noop` (#445)

`getKeyInfoContent` was incorrectly set to `noop` instead of `getCertFromKeyInfo`
Diff: diff --git a/src/signed-xml.ts b/src/signed-xml.ts
index ca19db7..e5d80af 100644
--- a/src/signed-xml.ts
+++ b/src/signed-xml.ts
@@ -147,12 +147,12 @@ export class SignedXml {
     } else if (utils.isArrayHasLength(inclusiveNamespacesPrefixList)) {
       this.inclusiveNamespacesPrefixList = inclusiveNamespacesPrefixList;
     }
     this.implicitTransforms = implicitTransforms ?? this.implicitTransforms;
     this.keyInfoAttributes = keyInfoAttributes ?? this.keyInfoAttributes;
-    this.getKeyInfoContent = getKeyInfoContent ?? SignedXml.noop;
-    this.getCertFromKeyInfo = getCertFromKeyInfo ?? this.getCertFromKeyInfo;
+    this.getKeyInfoContent = getKeyInfoContent ?? this.getKeyInfoContent;
+    this.getCertFromKeyInfo = getCertFromKeyInfo ?? SignedXml.noop;
     this.CanonicalizationAlgorithms;
     this.HashAlgorithms;
     this.SignatureAlgorithms;
   }
 
diff --git a/test/document-tests.spec.ts b/test/document-tests.spec.ts
index 0ade323..57b552a 100644
--- a/test/document-tests.spec.ts
+++ b/test/document-tests.spec.ts
@@ -45,10 +45,11 @@ describe("Document tests", function () {
 describe("Validated node references tests", function () {
   it("should return references if the document is validly signed", function () {
     const xml = fs.readFileSync("./test/static/valid_saml.xml", "utf-8");
     const doc = new xmldom.DOMParser().parseFromString(xml);
     const sig = new SignedXml();
+    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;
     sig.loadSignature(sig.findSignatures(doc)[0]);
     const validSignature = sig.checkSignature(xml);
     expect(validSignature).to.be.true;
 
     const ref = sig.getReferences()[0];
@@ -71,10 +72,11 @@ describe("Validated node references tests", function () {
 
   it("should return `null` if the selected node isn't found", function () {
     const xml = fs.readFileSync("./test/static/valid_saml.xml", "utf-8");
     const doc = new xmldom.DOMParser().parseFromString(xml);
     const sig = new SignedXml();
+    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;
     sig.loadSignature(sig.findSignatures(doc)[0]);
     const validSignature = sig.checkSignature(xml);
     expect(validSignature).to.be.true;
 
     const ref = sig.getReferences()[0];
@@ -84,10 +86,11 @@ describe("Validated node references tests", function () {
 
   it("should return the selected node if it is validly signed", function () {
     const xml = fs.readFileSync("./test/static/valid_saml.xml", "utf-8");
     const doc = new xmldom.DOMParser().parseFromString(xml);
     const sig = new SignedXml();
+    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;
     sig.loadSignature(sig.findSignatures(doc)[0]);
     const validSignature = sig.checkSignature(xml);
     expect(validSignature).to.be.true;
 
     const ref = sig.getReferences()[0];
diff --git a/test/key-info-tests.spec.ts b/test/key-info-tests.spec.ts
index 5e138e6..19c8f4a 100644
--- a/test/key-info-tests.spec.ts
+++ b/test/key-info-tests.spec.ts
@@ -11,11 +11,10 @@ describe("KeyInfo tests", function () {
     const sig = new SignedXml();
     sig.privateKey = fs.readFileSync("./test/static/client.pem");
     sig.publicCert = fs.readFileSync("./test/static/client_public.pem");
     sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#";
     sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
-    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
     sig.computeSignature(xml);
     const signedXml = sig.getSignedXml();
     const doc = new xmldom.DOMParser().parseFromString(signedXml);
     const x509 = xpath.select("//*[local-name(.)='X509Certificate']", doc.documentElement);
     isDomNode.assertIsArrayOfNodes(x509);
diff --git a/test/signature-unit-tests.spec.ts b/test/signature-unit-tests.spec.ts
index bbb0c9f..0db89a4 100644
--- a/test/signature-unit-tests.spec.ts
+++ b/test/signature-unit-tests.spec.ts
@@ -532,11 +532,10 @@ describe("Signature unit tests", function () {
     sig.SignatureAlgorithms["http://dummySignatureAlgorithm"] = DummySignatureAlgorithm;
 
     sig.signatureAlgorithm = "http://dummySignatureAlgorithm";
     sig.canonicalizationAlgorithm = "http://DummyCanonicalization";
     sig.privateKey = "";
-    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
 
     sig.addReference({
       xpath: "//*[local-name(.)='x']",
       transforms: ["http://DummyTransformation"],
       digestAlgorithm: "http://dummyDigest",
@@ -1235,11 +1234,10 @@ describe("Signature unit tests", function () {
     const pemBuffer = fs.readFileSync("./test/static/client_bundle.pem");
     sig.privateKey = pemBuffer;
     sig.publicCert = pemBuffer;
     sig.canonicalizationAlgorithm = "http://www.w3.org/2001/10/xml-exc-c14n#";
     sig.signatureAlgorithm = "http://www.w3.org/2000/09/xmldsig#rsa-sha1";
-    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
     sig.computeSignature(xml);
     const signedXml = sig.getSignedXml();
 
     const doc = new xmldom.DOMParser().parseFromString(signedXml);
 

Vulnerable code:     this.getKeyInfoContent = getKeyInfoContent ?? SignedXml.noop;
    this.getCertFromKeyInfo = getCertFromKeyInfo ?? this.getCertFromKeyInfo;
    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
    sig.getKeyInfoContent = SignedXml.getKeyInfoContent;
Fixed code:     this.getKeyInfoContent = getKeyInfoContent ?? this.getKeyInfoContent;
    this.getCertFromKeyInfo = getCertFromKeyInfo ?? SignedXml.noop;
    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;
    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;
    sig.getCertFromKeyInfo = SignedXml.getCertFromKeyInfo;