getSignedXml() writes a carriage return in text raw, so the signed document fails verification #587

Summary

If a document to sign has a carriage return in its text as a character reference (
 or 
, which is how documents that preserve CRLF line endings carry it), computeSignature() digests it correctly: canonicalization renders it as 
. getSignedXml() then serializes the document with xmldom's toString(), which writes that character raw. XML parsers turn a raw carriage return into a line feed (XML 1.0 §2.11). So the document getSignedXml() returns no longer holds the data that was signed, and no verifier, xml-crypto included, accepts it.

• Attribute values are unaffected: xmldom already writes 
 there.
• Verification is unaffected: a received document that carries the character reference parses back to a carriage return.
• Upgrading xmldom won't help: 0.8.15, which we depend on, and 0.9.12 both serialize text this way.

It fails closed, so it is not a security issue.

Reproduces on master @ 675263b and on 6.1.2. It is not #238, which is about verifying documents that contain literal CRLF line endings.

Reproduction

Save as repro.js in the repo root and run npm run build && node repro.js. It signs three documents and verifies each getSignedXml() result as returned, then again with every raw \r written as 
.

const fs = require("fs");
const { DOMParser } = require("@xmldom/xmldom");
const { SignedXml } = require("./lib/index.js");

function signAndVerify(label, xml) {
  const signer = new SignedXml({
    privateKey: fs.readFileSync("./test/static/client.pem"),
    canonicalizationAlgorithm: "http://www.w3.org/2001/10/xml-exc-c14n#",
    signatureAlgorithm: "http://www.w3.org/2001/04/xmldsig-more#rsa-sha256",
  });
  signer.addReference({
    xpath: "//item",
    transforms: ["http://www.w3.org/2001/10/xml-exc-c14n#"],
    digestAlgorithm: "http://www.w3.org/2001/04/xmlenc#sha256",
  });
  signer.computeSignature(xml);
  const signedXml = signer.getSignedXml();
  const item = signedXml.slice(signedXml.indexOf("<item"), signedXml.indexOf("<Signature"));
  console.log(`${label} getSignedXml(): ${JSON.stringify(item)}`);
  console.log(`  verifies as returned:           ${verify(signedXml)}`);
  console.log(`  verifies with \\r written &#xD;: ${verify(signedXml.replace(/\r/g, "&#xD;"))}`);
}

function verify(xml) {
  const verifier = new SignedXml({ publicCert: fs.readFileSync("./test/static/client_public.pem") });
  verifier.loadSignature(verifier.findSignatures(new DOMParser().parseFromString(xml))[0]);
  return verifier.checkSignature(xml);
}

signAndVerify("text &#13;     ", "<root><item>a&#13;b</item></root>");
signAndVerify("attribute &#13;", "<root><item note='a&#13;b'>ab</item></root>");
signAndVerify("text &#10;     ", "<root><item>a&#10;b</item></root>");

Actual output

text &#13;      getSignedXml(): "<item Id=\"_0\">a\rb</item>"
  verifies as returned:           false
  verifies with \r written &#xD;: true
attribute &#13; getSignedXml(): "<item note=\"a&#13;b\" Id=\"_0\">ab</item>"
  verifies as returned:           true
  verifies with \r written &#xD;: true
text &#10;      getSignedXml(): "<item Id=\"_0\">a\nb</item>"
  verifies as returned:           true
  verifies with \r written &#xD;: true

The failing verification reports invalid signature: for uri #_0 calculated digest is jt7nR6fGP7f+EBXulKVq4cP1g9hJGh0OX8VQ1AE9swE= but the xml to validate supplies digest /zbH+aeO9y2CtSEkjryxXdkjGu8TdXE3U4B/SARM+oQ=.

Expected output

getSignedXml() writes "<item Id=\"_0\">a&#xD;b</item>", and every line reports true.

Affected sites

Suggested fix

Write the carriage returns in text nodes as 
 when serializing the signed document.

computeSignature() only accepts a string, which xml-crypto parses itself, so a carriage return can reach the DOM only through a character reference. XML allows those only in text and attribute values, and xmldom already escapes attribute values. So while computeSignature() takes a string, replacing each raw \r in the doc.toString() output with 
 is exact. If it ever accepts a caller-built DOM, a carriage return could also sit in a comment, CDATA section or processing instruction, where it has no escape. At that point this needs a real serializer rather than a replace.

Suggested test coverage

• Signing a document with 
 in text, and with 
, returns XML that checkSignature() accepts, with the carriage return written as 
.