fix: escape comment and CDATA delimiters when building XML (GHSA-gh4j-gqv2-49f6)

spec/cdata_spec.js

@@ -462,4 +462,22 @@ patronymic</person></root>`;
         // console.log(JSON.stringify(result,null,4));
         expect(result).toEqual(expected);
     });
+
+    it("should neutralize CDATA delimiters when building so a value cannot break out (GHSA-gh4j-gqv2-49f6)", function() {
+        const payload = "a]]><script>alert(1)</script><![CDATA[b";
+        const stripCData = (s) => s.replace(/<!\[CDATA\[[\s\S]*?\]\]>/g, "");
+
+        const out1 = new XMLBuilder({ cdataPropName: "#cdata", format: false })
+            .build({ data: { "#cdata": payload } });
+        expect(XMLValidator.validate("<r>" + out1 + "</r>")).toBe(true);
+        expect(stripCData(out1)).not.toContain("<script>");
+        // the split-CDATA escaping is lossless: the (possibly split) CDATA text concatenates back to the original
+        const roundTrip = new XMLParser({ cdataPropName: "#cdata" }).parse(out1).data["#cdata"];
+        expect([].concat(roundTrip).join("")).toEqual(payload);
+
+        const out2 = new XMLBuilder({ preserveOrder: true, cdataPropName: "#cdata", format: false })
+            .build([{ "#cdata": [{ "#text": payload }] }]);
+        expect(XMLValidator.validate("<r>" + out2 + "</r>")).toBe(true);
+        expect(stripCData(out2)).not.toContain("<script>");
+    });
 });

spec/comments_spec.js

@@ -81,5 +81,20 @@ it("should build XML with Comments without parseOrder", function() {
   expect(xmlOutput.replace(/\s+/g, "")).toEqual(expected.replace(/\s+/g, ""));
 });
 
+it("should neutralize comment delimiters when building so a value cannot break out (GHSA-gh4j-gqv2-49f6)", function() {
+  const payload = "a--><script>alert(1)</script><!--b";
+  const stripComments = (s) => s.replace(/<!--[\s\S]*?-->/g, "");
+
+  const out1 = new XMLBuilder({ commentPropName: "#comment", format: false })
+    .build({ note: { "#comment": payload } });
+  expect(XMLValidator.validate("<r>" + out1 + "</r>")).toBe(true);
+  expect(stripComments(out1)).not.toContain("<script>");
+
+  const out2 = new XMLBuilder({ preserveOrder: true, commentPropName: "#comment", format: false })
+    .build([{ "#comment": [{ "#text": payload }] }]);
+  expect(XMLValidator.validate("<r>" + out2 + "</r>")).toBe(true);
+  expect(stripComments(out2)).not.toContain("<script>");
+});
+
 });
 

src/xmlbuilder/json2xml.js

@@ -206,7 +206,10 @@ Builder.prototype.buildObjectNode = function(val, key, attrStr, level) {
     if ((attrStr || attrStr === '') && val.indexOf('<') === -1) {
       return ( this.indentate(level) + '<' +  key + attrStr + piClosingChar + '>' + val + tagEndExp );
     } else if (this.options.commentPropName !== false && key === this.options.commentPropName && piClosingChar.length === 0) {
-      return this.indentate(level) + `<!--${val}-->` + this.newLine;
+      const safeVal = String(val)
+        .replace(/--/g, '- -')   // -- is illegal anywhere in comment content
+        .replace(/-$/, '- ');    // trailing - would form --> with the closing delimiter
+      return this.indentate(level) + `<!--${safeVal}-->` + this.newLine;
     }else {
       return (
         this.indentate(level) + '<' + key + attrStr + piClosingChar + this.tagEndChar +
@@ -242,9 +245,13 @@ function buildEmptyObjNode(val, key, attrStr, level) {
 
 Builder.prototype.buildTextValNode = function(val, key, attrStr, level) {
   if (this.options.cdataPropName !== false && key === this.options.cdataPropName) {
-    return this.indentate(level) + `<![CDATA[${val}]]>` +  this.newLine;
+    const safeVal = String(val).replace(/\]\]>/g, ']]]]><![CDATA[>');
+    return this.indentate(level) + `<![CDATA[${safeVal}]]>` +  this.newLine;
   }else if (this.options.commentPropName !== false && key === this.options.commentPropName) {
-    return this.indentate(level) + `<!--${val}-->` +  this.newLine;
+    const safeVal = String(val)
+      .replace(/--/g, '- -')   // -- is illegal anywhere in comment content
+      .replace(/-$/, '- ');    // trailing - would form --> with the closing delimiter
+    return this.indentate(level) + `<!--${safeVal}-->` +  this.newLine;
   }else if(key[0] === "?") {//PI tag
     return  this.indentate(level) + '<' + key + attrStr+ '?' + this.tagEndChar; 
   }else{

src/xmlbuilder/orderedJs2Xml.js

@@ -54,11 +54,15 @@ function arrToStr(arr, options, jPath, indentation) {
             if (isPreviousElementTag) {
                 xmlStr += indentation;
             }
-            xmlStr += `<![CDATA[${tagObj[tagName][0][options.textNodeName]}]]>`;
+            const cdataVal = String(tagObj[tagName][0][options.textNodeName]).replace(/\]\]>/g, ']]]]><![CDATA[>');
+            xmlStr += `<![CDATA[${cdataVal}]]>`;
             isPreviousElementTag = false;
             continue;
         } else if (tagName === options.commentPropName) {
-            xmlStr += indentation + `<!--${tagObj[tagName][0][options.textNodeName]}-->`;
+            const commentVal = String(tagObj[tagName][0][options.textNodeName])
+                .replace(/--/g, '- -')   // -- is illegal anywhere in comment content
+                .replace(/-$/, '- ');    // trailing - would form --> with the closing delimiter
+            xmlStr += indentation + `<!--${commentVal}-->`;
             isPreviousElementTag = true;
             continue;
         } else if (tagName[0] === "?") {