Skip to content

Address CVEs: CVE-2025-29774 and CVE-2025-29775 #494

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
cjbarth merged 1 commit into from
Apr 9, 2025
Merged

Address CVEs: CVE-2025-29774 and CVE-2025-29775 #494

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
116 changes: 103 additions & 13 deletions src/signed-xml.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -252,26 +252,72 @@ export class SignedXml {
this.signedXml = xml;

const doc = new xmldom.DOMParser().parseFromString(xml);
// Reset the references as only references from our re-parsed signedInfo node can be trusted
this.references = [];

const unverifiedSignedInfoCanon = this.getCanonSignedInfoXml(doc);
if (!unverifiedSignedInfoCanon) {
if (callback) {
callback(new Error("Canonical signed info cannot be empty"), false);
return;
}

throw new Error("Canonical signed info cannot be empty");
}

// unsigned, verify later to keep with consistent callback behavior
const parsedUnverifiedSignedInfo = new xmldom.DOMParser().parseFromString(
unverifiedSignedInfoCanon,
"text/xml",
);

const unverifiedSignedInfoDoc = parsedUnverifiedSignedInfo.documentElement;
if (!unverifiedSignedInfoDoc) {
if (callback) {
callback(new Error("Could not parse unverifiedSignedInfoCanon into a document"), false);
return;
}

throw new Error("Could not parse unverifiedSignedInfoCanon into a document");
}

const references = utils.findChildren(unverifiedSignedInfoDoc, "Reference");
if (!utils.isArrayHasLength(references)) {
if (callback) {
callback(new Error("could not find any Reference elements"), false);
return;
}

throw new Error("could not find any Reference elements");
}

// TODO: In a future release we'd like to load the Signature and its References at the same time,
// however, in the `.loadSignature()` method we don't have the entire document,
// which we need to to keep the inclusive namespaces
for (const reference of references) {
this.loadReference(reference);
}

if (!this.getReferences().every((ref) => this.validateReference(ref, doc))) {
if (callback) {
callback(new Error("Could not validate all references"));
callback(new Error("Could not validate all references"), false);
return;
}

return false;
}

const signedInfoCanon = this.getCanonSignedInfoXml(doc);
// Stage B: Take the signature algorithm and key and verify the SignatureValue against the canonicalized SignedInfo
const signer = this.findSignatureAlgorithm(this.signatureAlgorithm);
const key = this.getCertFromKeyInfo(this.keyInfo) || this.publicCert || this.privateKey;
if (key == null) {
throw new Error("KeyInfo or publicCert or privateKey is required to validate signature");
}

if (callback) {
signer.verifySignature(signedInfoCanon, key, this.signatureValue, callback);
signer.verifySignature(unverifiedSignedInfoCanon, key, this.signatureValue, callback);
} else {
const verified = signer.verifySignature(signedInfoCanon, key, this.signatureValue);
const verified = signer.verifySignature(unverifiedSignedInfoCanon, key, this.signatureValue);

if (verified === false) {
throw new Error(
Expand All @@ -295,6 +341,11 @@ export class SignedXml {
if (signedInfo.length === 0) {
throw new Error("could not find SignedInfo element in the message");
}
if (signedInfo.length > 1) {
throw new Error(
"could not get canonicalized signed info for a signature that contains multiple SignedInfo nodes",
);
}

if (
this.canonicalizationAlgorithm === "http://www.w3.org/TR/2001/REC-xml-c14n-20010315" ||
Expand Down Expand Up @@ -522,11 +573,43 @@ export class SignedXml {
this.signatureAlgorithm = signatureAlgorithm.value as SignatureAlgorithmType;
}

this.references = [];
const references = xpath.select(
".//*[local-name(.)='SignedInfo']/*[local-name(.)='Reference']",
signatureNode,
const signedInfoNodes = utils.findChildren(this.signatureNode, "SignedInfo");
if (!utils.isArrayHasLength(signedInfoNodes)) {
throw new Error("no signed info node found");
}
if (signedInfoNodes.length > 1) {
throw new Error("could not load signature that contains multiple SignedInfo nodes");
}

// Try to operate on the c14n version of `signedInfo`. This forces the initial `getReferences()`
// API call to always return references that are loaded under the canonical `SignedInfo`
// in the case that the client access the `.references` **before** signature verification.

// Ensure canonicalization algorithm is exclusive, otherwise we'd need the entire document
let canonicalizationAlgorithmForSignedInfo = this.canonicalizationAlgorithm;
if (
!canonicalizationAlgorithmForSignedInfo ||
canonicalizationAlgorithmForSignedInfo ===
"http://www.w3.org/TR/2001/REC-xml-c14n-20010315" ||
canonicalizationAlgorithmForSignedInfo ===
"http://www.w3.org/TR/2001/REC-xml-c14n-20010315#WithComments"
) {
canonicalizationAlgorithmForSignedInfo = "http://www.w3.org/2001/10/xml-exc-c14n#";
}

const temporaryCanonSignedInfo = this.getCanonXml(
[canonicalizationAlgorithmForSignedInfo],
signedInfoNodes[0],
);
const temporaryCanonSignedInfoXml = new xmldom.DOMParser().parseFromString(
temporaryCanonSignedInfo,
"text/xml",
);
const signedInfoDoc = temporaryCanonSignedInfoXml.documentElement;

this.references = [];
const references = utils.findChildren(signedInfoDoc, "Reference");

if (!utils.isArrayHasLength(references)) {
throw new Error("could not find any Reference elements");
}
Expand Down Expand Up @@ -572,11 +655,15 @@ export class SignedXml {
if (nodes.length === 0) {
throw new Error(`could not find DigestValue node in reference ${refNode.toString()}`);
}
const firstChild = nodes[0].firstChild;
if (!firstChild || !("data" in firstChild)) {
throw new Error(`could not find the value of DigestValue in ${nodes[0].toString()}`);
if (nodes.length > 1) {
throw new Error(
`could not load reference for a node that contains multiple DigestValue nodes: ${refNode.toString()}`,
);
}
const digestValue = nodes[0].textContent;
if (!digestValue) {
throw new Error(`could not find the value of DigestValue in ${refNode.toString()}`);
}
const digestValue = firstChild.data;

const transforms: string[] = [];
let inclusiveNamespacesPrefixList: string[] = [];
Expand Down Expand Up @@ -626,11 +713,14 @@ export class SignedXml {
) {
transforms.push("http://www.w3.org/TR/2001/REC-xml-c14n-20010315");
}
const refUri = isDomNode.isElementNode(refNode)
? refNode.getAttribute("URI") || undefined
: undefined;

this.addReference({
transforms,
digestAlgorithm: digestAlgo,
uri: isDomNode.isElementNode(refNode) ? utils.findAttr(refNode, "URI")?.value : undefined,
uri: refUri,
digestValue,
inclusiveNamespacesPrefixList,
isEmptyUri: false,
Expand Down
74 changes: 74 additions & 0 deletions test/saml-response-tests.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,78 @@ describe("SAML response tests", function () {
// This doesn't matter, just want to make sure that we don't fail due to unknown algorithm
expect(() => sig.checkSignature(xml)).to.throw(/^invalid signature/);
});

it("throws an error for a document with no `SignedInfo` node", function () {
const xml = fs.readFileSync("./test/static/invalid_saml_no_signed_info.xml", "utf-8");
const doc = new xmldom.DOMParser().parseFromString(xml);
const node = xpath.select1(
"/*/*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
doc,
);

isDomNode.assertIsNodeLike(node);
const sig = new SignedXml();
const feidePublicCert = fs.readFileSync("./test/static/feide_public.pem");
sig.publicCert = feidePublicCert;

expect(() => sig.loadSignature(node)).to.throw("no signed info node found");
});

it("test validation ignores an additional wrapped `SignedInfo` node", function () {
const xml = fs.readFileSync("./test/static/saml_wrapped_signed_info_node.xml", "utf-8");
const doc = new xmldom.DOMParser().parseFromString(xml);
const assertion = xpath.select1("//*[local-name(.)='Assertion']", doc);
isDomNode.assertIsNodeLike(assertion);
const signature = xpath.select1(
"//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
assertion,
);
isDomNode.assertIsNodeLike(signature);

const sig = new SignedXml();
sig.publicCert = fs.readFileSync("./test/static/saml_external_ns.pem");
sig.loadSignature(signature);
expect(sig.getReferences().length).to.equal(1);
const checkSignatureResult = sig.checkSignature(xml);
expect(checkSignatureResult).to.be.true;
});

it("test signature throws if multiple `SignedInfo` nodes are found", function () {
const xml = fs.readFileSync("./test/static/saml_multiple_signed_info_nodes.xml", "utf-8");
const doc = new xmldom.DOMParser().parseFromString(xml);
const assertion = xpath.select1("//*[local-name(.)='Assertion'][1]", doc);
isDomNode.assertIsNodeLike(assertion);
const signature = xpath.select1(
"//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
assertion,
);
isDomNode.assertIsNodeLike(signature);

const sig = new SignedXml();
sig.publicCert = fs.readFileSync("./test/static/saml_external_ns.pem");
expect(() => sig.loadSignature(signature)).to.throw(
"could not load signature that contains multiple SignedInfo nodes",
);
});

describe("for a SAML response with a digest value comment", () => {
it("loads digest value from text content instead of comment", function () {
const xml = fs.readFileSync("./test/static/valid_saml_with_digest_comment.xml", "utf-8");
const doc = new xmldom.DOMParser().parseFromString(xml);
const assertion = xpath.select1("//*[local-name(.)='Assertion']", doc);
isDomNode.assertIsNodeLike(assertion);
const signature = xpath.select1(
"//*[local-name(.)='Signature' and namespace-uri(.)='http://www.w3.org/2000/09/xmldsig#']",
assertion,
);
isDomNode.assertIsNodeLike(signature);
const sig = new SignedXml();
sig.publicCert = fs.readFileSync("./test/static/feide_public.pem");

sig.loadSignature(signature);

expect(sig.getReferences()[0].digestValue).to.equal("RnNjoyUguwze5w2R+cboyTHlkQk=");
expect(sig.checkSignature(xml)).to.be.false;
});
});
});
9 changes: 9 additions & 0 deletions test/static/invalid_saml_no_signed_info.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
<samlp:Response xmlns:samlp="urn:oasis:names:tc:SAML:2.0:protocol" xmlns:saml="urn:oasis:names:tc:SAML:2.0:assertion" ID="pfx94e4a319-b6f7-4a40-25d1-01fcb642e4c5" Version="2.0" IssueInstant="2012-07-03T11:32:20Z" Destination="http://localhost:3000/login/callback" InResponseTo="_d766d16611ac0d14121b"><saml:Issuer>https://openidp.feide.no</saml:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ds:SignatureValue>dkONrkxW+LSuDvnNMG/mWYFa47d2WGyapLhXSTYqrlT9Td+tT7ciojNJ55WTaPaCMt7IrGtIxxskPAZIjdIn5pRyDxHr0joWxzZ7oZHCOI1CnQV5HjOq+rzzmEN2LctCZ6S4hbL7SQ1qJ3vp2BCXAygy4tmJOURQdnk0KLwwRS8=</ds:SignatureValue>
<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICizCCAfQCCQCY8tKaMc0BMjANBgkqhkiG9w0BAQUFADCBiTELMAkGA1UEBhMCTk8xEjAQBgNVBAgTCVRyb25kaGVpbTEQMA4GA1UEChMHVU5JTkVUVDEOMAwGA1UECxMFRmVpZGUxGTAXBgNVBAMTEG9wZW5pZHAuZmVpZGUubm8xKTAnBgkqhkiG9w0BCQEWGmFuZHJlYXMuc29sYmVyZ0B1bmluZXR0Lm5vMB4XDTA4MDUwODA5MjI0OFoXDTM1MDkyMzA5MjI0OFowgYkxCzAJBgNVBAYTAk5PMRIwEAYDVQQIEwlUcm9uZGhlaW0xEDAOBgNVBAoTB1VOSU5FVFQxDjAMBgNVBAsTBUZlaWRlMRkwFwYDVQQDExBvcGVuaWRwLmZlaWRlLm5vMSkwJwYJKoZIhvcNAQkBFhphbmRyZWFzLnNvbGJlcmdAdW5pbmV0dC5ubzCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAt8jLoqI1VTlxAZ2axiDIThWcAOXdu8KkVUWaN/SooO9O0QQ7KRUjSGKN9JK65AFRDXQkWPAu4HlnO4noYlFSLnYyDxI66LCr71x4lgFJjqLeAvB/GqBqFfIZ3YK/NrhnUqFwZu63nLrZjcUZxNaPjOOSRSDaXpv1kb5k3jOiSGECAwEAATANBgkqhkiG9w0BAQUFAAOBgQBQYj4cAafWaYfjBU2zi1ElwStIaJ5nyp/s/8B8SAPK2T79McMyccP3wSW13LHkmM1jwKe3ACFXBvqGQN0IbcH49hu0FKhYFM/GPDJcIHFBsiyMBXChpye9vBaTNEBCtU3KjjyG0hRT2mAQ9h+bkPmOvlEo/aH0xR68Z9hw4PF13w==</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><samlp:Status><samlp:StatusCode Value="urn:oasis:names:tc:SAML:2.0:status:Success"/></samlp:Status><saml:Assertion xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xmlns:xs="http://www.w3.org/2001/XMLSchema" ID="pfx66496e6c-3c29-230d-6d47-b245434b872d" Version="2.0" IssueInstant="2012-07-03T11:32:20Z"><saml:Issuer>https://openidp.feide.no</saml:Issuer><ds:Signature xmlns:ds="http://www.w3.org/2000/09/xmldsig#">
<ds:SignedInfo><ds:CanonicalizationMethod Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/>
<ds:SignatureMethod Algorithm="http://www.w3.org/2000/09/xmldsig#rsa-sha1"/>
<ds:Reference URI="#pfx66496e6c-3c29-230d-6d47-b245434b872d"><ds:Transforms><ds:Transform Algorithm="http://www.w3.org/2000/09/xmldsig#enveloped-signature"/><ds:Transform Algorithm="http://www.w3.org/2001/10/xml-exc-c14n#"/></ds:Transforms><ds:DigestMethod Algorithm="http://www.w3.org/2000/09/xmldsig#sha1"/><ds:DigestValue>RnNjoyUguwze5w2R+cboyTHlkQk=</ds:DigestValue></ds:Reference></ds:SignedInfo><ds:SignatureValue>aw5711jKP7xragunjRRCAD4mT4xKHc37iohBpQDbdSomD3ksOSB96UZQp0MtaC3xlVSkMtYw85Om96T2q2xrxLLYVA50eFJEMMF7SCVPStWTVjBlaCuOPEQxIaHyJs9Sy3MCEfbBh4Pqn9IJBd1kzwdlCrWWjAmksbFFg5wHQJA=</ds:SignatureValue>
<ds:KeyInfo><ds:X509Data><ds:X509Certificate>MIICizCCAfQCCQCY8tKaMc0BMjANBgkqhkiG9w0BAQUFADCBiTELMAkGA1UEBhMCTk8xEjAQBgNVBAgTCVRyb25kaGVpbTEQMA4GA1UEChMHVU5JTkVUVDEOMAwGA1UECxMFRmVpZGUxGTAXBgNVBAMTEG9wZW5pZHAuZmVpZGUubm8xKTAnBgkqhkiG9w0BCQEWGmFuZHJlYXMuc29sYmVyZ0B1bmluZXR0Lm5vMB4XDTA4MDUwODA5MjI0OFoXDTM1MDkyMzA5MjI0OFowgYkxCzAJBgNVBAYTAk5PMRIwEAYDVQQIEwlUcm9uZGhlaW0xEDAOBgNVBAoTB1VOSU5FVFQxDjAMBgNVBAsTBUZlaWRlMRkwFwYDVQQDExBvcGVuaWRwLmZlaWRlLm5vMSkwJwYJKoZIhvcNAQkBFhphbmRyZWFzLnNvbGJlcmdAdW5pbmV0dC5ubzCBnzANBgkqhkiG9w0BAQEFAAOBjQAwgYkCgYEAt8jLoqI1VTlxAZ2axiDIThWcAOXdu8KkVUWaN/SooO9O0QQ7KRUjSGKN9JK65AFRDXQkWPAu4HlnO4noYlFSLnYyDxI66LCr71x4lgFJjqLeAvB/GqBqFfIZ3YK/NrhnUqFwZu63nLrZjcUZxNaPjOOSRSDaXpv1kb5k3jOiSGECAwEAATANBgkqhkiG9w0BAQUFAAOBgQBQYj4cAafWaYfjBU2zi1ElwStIaJ5nyp/s/8B8SAPK2T79McMyccP3wSW13LHkmM1jwKe3ACFXBvqGQN0IbcH49hu0FKhYFM/GPDJcIHFBsiyMBXChpye9vBaTNEBCtU3KjjyG0hRT2mAQ9h+bkPmOvlEo/aH0xR68Z9hw4PF13w==</ds:X509Certificate></ds:X509Data></ds:KeyInfo></ds:Signature><saml:Subject><saml:NameID SPNameQualifier="passport-saml" Format="urn:oasis:names:tc:SAML:2.0:nameid-format:transient">_6c5dcaa3053321ff4d63785fbc3f67c59a129cde82</saml:NameID><saml:SubjectConfirmation Method="urn:oasis:names:tc:SAML:2.0:cm:bearer"><saml:SubjectConfirmationData NotOnOrAfter="2012-07-03T11:37:20Z" Recipient="http://localhost:3000/login/callback" InResponseTo="_d766d16611ac0d14121b"/></saml:SubjectConfirmation></saml:Subject><saml:Conditions NotBefore="2012-07-03T11:31:50Z" NotOnOrAfter="2012-07-03T11:37:20Z"><saml:AudienceRestriction><saml:Audience>passport-saml</saml:Audience></saml:AudienceRestriction></saml:Conditions><saml:AuthnStatement AuthnInstant="2012-07-03T11:32:20Z" SessionNotOnOrAfter="2012-07-03T19:32:20Z" SessionIndex="_c8e6823fe38ddbce125f9be6e5118b8c352d04bcae"><saml:AuthnContext><saml:AuthnContextClassRef>urn:oasis:names:tc:SAML:2.0:ac:classes:Password</saml:AuthnContextClassRef></saml:AuthnContext></saml:AuthnStatement><saml:AttributeStatement><saml:Attribute Name="uid" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">bergie</saml:AttributeValue></saml:Attribute><saml:Attribute Name="givenName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Henri</saml:AttributeValue></saml:Attribute><saml:Attribute Name="sn" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Bergius</saml:AttributeValue></saml:Attribute><saml:Attribute Name="cn" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Henri Bergius</saml:AttributeValue></saml:Attribute><saml:Attribute Name="mail" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="eduPersonPrincipalName" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="eduPersonTargetedID" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">8216c78fe244502efa13f62e6615c94acb7bdf3e</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:0.9.2342.19200300.100.1.1" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">bergie</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:2.5.4.42" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Henri</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:2.5.4.4" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Bergius</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:2.5.4.3" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">Henri Bergius</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:0.9.2342.19200300.100.1.3" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.6" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">[email protected]</saml:AttributeValue></saml:Attribute><saml:Attribute Name="urn:oid:1.3.6.1.4.1.5923.1.1.1.10" NameFormat="urn:oasis:names:tc:SAML:2.0:attrname-format:uri"><saml:AttributeValue xsi:type="xs:string">8216c78fe244502efa13f62e6615c94acb7bdf3e</saml:AttributeValue></saml:Attribute></saml:AttributeStatement></saml:Assertion></samlp:Response>
Loading