Skip to content

Commit 04a1de4

Browse files
committed
fix: prevent signature self-reference
1 parent 230d4c5 commit 04a1de4

File tree

3 files changed

+121
-34
lines changed

3 files changed

+121
-34
lines changed

src/signed-xml.ts

Lines changed: 35 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@ import * as execC14n from "./exclusive-canonicalization";
2626
import * as hashAlgorithms from "./hash-algorithms";
2727
import * as signatureAlgorithms from "./signature-algorithms";
2828
import * as utils from "./utils";
29+
import { isDescendantOf } from "./utils";
2930

3031
export class SignedXml {
3132
idMode?: "wssecurity";
@@ -992,8 +993,9 @@ export class SignedXml {
992993
const nodeXml = new xmldom.DOMParser().parseFromString(dummySignatureWrapper);
993994

994995
// Because we are using a dummy wrapper hack described above, we know there will be a `firstChild`
996+
// and that it will be an `Element` node.
995997
// eslint-disable-next-line @typescript-eslint/no-non-null-assertion
996-
const signatureDoc = nodeXml.documentElement.firstChild!;
998+
const signatureElem = nodeXml.documentElement.firstChild! as Element;
997999

9981000
const referenceNode = xpath.select1(location.reference, doc);
9991001

@@ -1010,29 +1012,29 @@ export class SignedXml {
10101012
}
10111013

10121014
if (location.action === "append") {
1013-
referenceNode.appendChild(signatureDoc);
1015+
referenceNode.appendChild(signatureElem);
10141016
} else if (location.action === "prepend") {
1015-
referenceNode.insertBefore(signatureDoc, referenceNode.firstChild);
1017+
referenceNode.insertBefore(signatureElem, referenceNode.firstChild);
10161018
} else if (location.action === "before") {
10171019
if (referenceNode.parentNode == null) {
10181020
throw new Error(
10191021
"`location.reference` refers to the root node (by default), so we can't insert `before`",
10201022
);
10211023
}
1022-
referenceNode.parentNode.insertBefore(signatureDoc, referenceNode);
1024+
referenceNode.parentNode.insertBefore(signatureElem, referenceNode);
10231025
} else if (location.action === "after") {
10241026
if (referenceNode.parentNode == null) {
10251027
throw new Error(
10261028
"`location.reference` refers to the root node (by default), so we can't insert `after`",
10271029
);
10281030
}
1029-
referenceNode.parentNode.insertBefore(signatureDoc, referenceNode.nextSibling);
1031+
referenceNode.parentNode.insertBefore(signatureElem, referenceNode.nextSibling);
10301032
}
10311033

10321034
// Process any signature references after the signature has been added to the document
1033-
this.processSignatureReferences(doc, prefix);
1035+
this.processSignatureReferences(doc, signatureElem, prefix);
10341036

1035-
this.signatureNode = signatureDoc;
1037+
this.signatureNode = signatureElem;
10361038
const signedInfoNodes = utils.findChildren(this.signatureNode, "SignedInfo");
10371039
if (signedInfoNodes.length === 0) {
10381040
const err3 = new Error("could not find SignedInfo element in the message");
@@ -1052,17 +1054,17 @@ export class SignedXml {
10521054
callback(err);
10531055
} else {
10541056
this.signatureValue = signature || "";
1055-
signatureDoc.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling);
1056-
this.signatureXml = signatureDoc.toString();
1057+
signatureElem.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling);
1058+
this.signatureXml = signatureElem.toString();
10571059
this.signedXml = doc.toString();
10581060
callback(null, this);
10591061
}
10601062
});
10611063
} else {
10621064
// Synchronous flow
10631065
this.calculateSignatureValue(doc);
1064-
signatureDoc.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling);
1065-
this.signatureXml = signatureDoc.toString();
1066+
signatureElem.insertBefore(this.createSignature(prefix), signedInfoNode.nextSibling);
1067+
this.signatureXml = signatureElem.toString();
10661068
this.signedXml = doc.toString();
10671069
}
10681070
}
@@ -1325,7 +1327,7 @@ export class SignedXml {
13251327
* Process references that weren't found in the initial document
13261328
* This is called after the initial signature has been created to handle references to signature elements
13271329
*/
1328-
private processSignatureReferences(signatureDoc: Document, prefix?: string) {
1330+
private processSignatureReferences(doc: Document, signatureElem: Element, prefix?: string) {
13291331
// Get unprocessed references
13301332
const unprocessedReferences = this.references.filter((ref) => !ref.wasProcessed);
13311333
if (unprocessedReferences.length === 0) {
@@ -1338,16 +1340,16 @@ export class SignedXml {
13381340

13391341
// Find the SignedInfo element to append to
13401342
const signedInfoNode = xpath.select1(
1341-
`.//*[local-name(.)='SignedInfo']`,
1342-
signatureDoc,
1343+
`./*[local-name(.)='SignedInfo']`,
1344+
signatureElem,
13431345
) as Element;
13441346
if (!signedInfoNode) {
13451347
throw new Error("Could not find SignedInfo element in signature");
13461348
}
13471349

13481350
// Process each unprocessed reference
13491351
for (const ref of unprocessedReferences) {
1350-
const nodes = xpath.selectWithResolver(ref.xpath ?? "", signatureDoc, this.namespaceResolver);
1352+
const nodes = xpath.selectWithResolver(ref.xpath ?? "", doc, this.namespaceResolver);
13511353

13521354
if (!utils.isArrayHasLength(nodes)) {
13531355
throw new Error(
@@ -1357,8 +1359,19 @@ export class SignedXml {
13571359

13581360
// Process the reference
13591361
for (const node of nodes) {
1362+
// Must not be a reference to Signature, SignedInfo, or a child of SignedInfo
1363+
if (
1364+
node === signatureElem ||
1365+
node === signedInfoNode ||
1366+
isDescendantOf(node, signedInfoNode)
1367+
) {
1368+
throw new Error(
1369+
`Cannot sign a reference to the Signature or SignedInfo element itself: ${ref.xpath}`,
1370+
);
1371+
}
1372+
13601373
// Create the reference element directly using DOM methods to avoid namespace issues
1361-
const referenceElem = signatureDoc.createElementNS(
1374+
const referenceElem = signatureElem.ownerDocument.createElementNS(
13621375
signatureNamespace,
13631376
`${prefix}Reference`,
13641377
);
@@ -1378,21 +1391,15 @@ export class SignedXml {
13781391
referenceElem.setAttribute("Type", ref.type);
13791392
}
13801393

1381-
const transformsElem = signatureDoc.createElementNS(
1382-
signatureNamespace,
1383-
`${prefix}Transforms`,
1384-
);
1394+
const transformsElem = doc.createElementNS(signatureNamespace, `${prefix}Transforms`);
13851395

13861396
for (const trans of ref.transforms || []) {
13871397
const transform = this.findCanonicalizationAlgorithm(trans);
1388-
const transformElem = signatureDoc.createElementNS(
1389-
signatureNamespace,
1390-
`${prefix}Transform`,
1391-
);
1398+
const transformElem = doc.createElementNS(signatureNamespace, `${prefix}Transform`);
13921399
transformElem.setAttribute("Algorithm", transform.getAlgorithmName());
13931400

13941401
if (utils.isArrayHasLength(ref.inclusiveNamespacesPrefixList)) {
1395-
const inclusiveNamespacesElem = signatureDoc.createElementNS(
1402+
const inclusiveNamespacesElem = doc.createElementNS(
13961403
transform.getAlgorithmName(),
13971404
"InclusiveNamespaces",
13981405
);
@@ -1407,21 +1414,15 @@ export class SignedXml {
14071414
}
14081415

14091416
// Get the canonicalized XML
1410-
const canonXml = this.getCanonReferenceXml(signatureDoc, ref, node);
1417+
const canonXml = this.getCanonReferenceXml(doc, ref, node);
14111418

14121419
// Get the digest algorithm and compute the digest value
14131420
const digestAlgorithm = this.findHashAlgorithm(ref.digestAlgorithm);
14141421

1415-
const digestMethodElem = signatureDoc.createElementNS(
1416-
signatureNamespace,
1417-
`${prefix}DigestMethod`,
1418-
);
1422+
const digestMethodElem = doc.createElementNS(signatureNamespace, `${prefix}DigestMethod`);
14191423
digestMethodElem.setAttribute("Algorithm", digestAlgorithm.getAlgorithmName());
14201424

1421-
const digestValueElem = signatureDoc.createElementNS(
1422-
signatureNamespace,
1423-
`${prefix}DigestValue`,
1424-
);
1425+
const digestValueElem = doc.createElementNS(signatureNamespace, `${prefix}DigestValue`);
14251426
digestValueElem.textContent = digestAlgorithm.getHash(canonXml);
14261427

14271428
referenceElem.appendChild(transformsElem);

src/utils.ts

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -313,3 +313,21 @@ export function validateDigestValue(digest, expectedDigest) {
313313

314314
return true;
315315
}
316+
317+
// Check if the given node is descendant of the given parent node
318+
export function isDescendantOf(node: Node, parent: Node): boolean {
319+
if (!node || !parent) {
320+
return false;
321+
}
322+
323+
let currentNode: Node | null = node.parentNode;
324+
325+
while (currentNode) {
326+
if (currentNode === parent) {
327+
return true;
328+
}
329+
currentNode = currentNode.parentNode;
330+
}
331+
332+
return false;
333+
}

test/signature-object-tests.spec.ts

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -450,3 +450,71 @@ describe("XAdES Object support in XML signatures", function () {
450450
expect(valid, errorMessage).to.be.true;
451451
});
452452
});
453+
454+
describe("Signature self-reference prevention", function () {
455+
it("should not allow self-referencing the Signature element", function () {
456+
const xml = "<root></root>";
457+
458+
const sig = new SignedXml({
459+
privateKey,
460+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
461+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
462+
});
463+
464+
sig.addReference({
465+
xpath: ".//*[local-name(.)='Signature']",
466+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
467+
transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
468+
});
469+
470+
expect(() => {
471+
sig.computeSignature(xml);
472+
}).to.throw(/Cannot sign a reference to the Signature or SignedInfo element itself/);
473+
});
474+
475+
it("should not allow self-referencing the SignedInfo element", function () {
476+
const xml = "<root></root>";
477+
478+
const sig = new SignedXml({
479+
privateKey,
480+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
481+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
482+
});
483+
484+
sig.addReference({
485+
xpath: ".//*[local-name(.)='SignedInfo']",
486+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
487+
transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
488+
});
489+
490+
expect(() => {
491+
sig.computeSignature(xml);
492+
}).to.throw(/Cannot sign a reference to the Signature or SignedInfo element itself/);
493+
});
494+
495+
it("should not allow signing children of the SignedInfo element", function () {
496+
const xml = "<root></root>";
497+
498+
const sig = new SignedXml({
499+
privateKey,
500+
canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
501+
signatureAlgorithm: "http://www.w3.org/2000/09/xmldsig#rsa-sha1",
502+
});
503+
504+
sig.addReference({
505+
xpath: "/*",
506+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
507+
transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
508+
});
509+
510+
sig.addReference({
511+
xpath: ".//*[local-name(.)='Reference']/*",
512+
digestAlgorithm: "http://www.w3.org/2000/09/xmldsig#sha1",
513+
transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
514+
});
515+
516+
expect(() => {
517+
sig.computeSignature(xml);
518+
}).to.throw(/Cannot sign a reference to the Signature or SignedInfo element itself/);
519+
});
520+
});

0 commit comments

Comments
 (0)