From 5bd28e7500db9d9d25b0d3c9219f4b2039eb3d62 Mon Sep 17 00:00:00 2001
From: Jan Kowalleck <jan.kowalleck@gmail.com>
Date: Wed, 8 May 2024 17:05:16 +0200
Subject: [PATCH] refactor: XML validator explicitely harden against XXE
 injections (#1064)

## Changed
* The provided XML validation capabilities are hardened (via [#1064];
concerns [#1061])
This is considered a security measure concerning XML external entity
(XXE) injection.

[#1061]:
https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061
[#1064]:
https://github.com/CycloneDX/cyclonedx-javascript-library/pull/1064

----

This is not an actual change.
Per default, the XML validation capabilities were already secure in the
intended ways.
This is to prevent the fuckup like in the yanked v6.7.0

---------

Signed-off-by: Jan Kowalleck <jan.kowalleck@gmail.com>
---
 HISTORY.md                                    |  7 +++++
 src/validation/xmlValidator.node.ts           |  6 +++-
 tests/_data/xxe_flag.txt                      |  4 +++
 .../Validation.XmlValidator.test.js           | 31 +++++++++++++++++++
 4 files changed, 47 insertions(+), 1 deletion(-)
 create mode 100644 tests/_data/xxe_flag.txt

diff --git a/HISTORY.md b/HISTORY.md
index 256d7b434..7eddd21ba 100644
--- a/HISTORY.md
+++ b/HISTORY.md
@@ -6,6 +6,13 @@ All notable changes to this project will be documented in this file.
 
 <!-- add unreleased items here -->
 
+* Changed
+  * The provided XML validation capabilities are hardened (via [#1064]; concerns [#1061])  
+    This is considered a security measure concerning XML external entity (XXE) injection.
+
+[#1061]: https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061
+[#1064]: https://github.com/CycloneDX/cyclonedx-javascript-library/pull/1064
+
 ## 6.7.1 -- 2024-05-07
 
 Reverted v6.7.0, back to v6.6.1
diff --git a/src/validation/xmlValidator.node.ts b/src/validation/xmlValidator.node.ts
index 667657ad0..91f9e4b06 100644
--- a/src/validation/xmlValidator.node.ts
+++ b/src/validation/xmlValidator.node.ts
@@ -48,7 +48,11 @@ async function getParser (): Promise<typeof parseXml> {
 
 const xmlParseOptions: Readonly<ParserOptions> = Object.freeze({
   nonet: true,
-  compact: true
+  compact: true,
+  // explicitly prevent XXE
+  // see https://cheatsheetseries.owasp.org/cheatsheets/XML_External_Entity_Prevention_Cheat_Sheet.html
+  noent: false,
+  dtdload: false
 })
 
 export class XmlValidator extends BaseValidator {
diff --git a/tests/_data/xxe_flag.txt b/tests/_data/xxe_flag.txt
new file mode 100644
index 000000000..1e47dc1e1
--- /dev/null
+++ b/tests/_data/xxe_flag.txt
@@ -0,0 +1,4 @@
+This file is target of XXE injection tests.
+The flag is:
+
+vaiquia2zoo3Im8ro9zahNg5mohwipouka2xieweed6ahChei3doox2fek3ise0lmohju3loh5oDu7eigh3jaeR2aiph2Voo
diff --git a/tests/integration/Validation.XmlValidator.test.js b/tests/integration/Validation.XmlValidator.test.js
index 17a273aa8..fc54ee081 100644
--- a/tests/integration/Validation.XmlValidator.test.js
+++ b/tests/integration/Validation.XmlValidator.test.js
@@ -18,6 +18,10 @@ Copyright (c) OWASP Foundation. All Rights Reserved.
 */
 
 const assert = require('assert')
+const { realpathSync } = require('fs')
+const { join } = require('path')
+const { pathToFileURL } = require('url')
+
 const { describe, it } = require('mocha')
 
 let hasDep = true
@@ -99,5 +103,32 @@ describe('Validation.XmlValidator', () => {
       const validationError = await validator.validate(input)
       assert.strictEqual(validationError, null)
     })
+
+    it('is not affected by XXE injection', async () => {
+      const validator = new XmlValidator(version)
+      const input = `<?xml version="1.0" encoding="UTF-8"?>
+        <!DOCTYPE poc [
+          <!ENTITY flag SYSTEM "${pathToFileURL(realpathSync(join(__dirname, '..', '_data', 'xxe_flag.txt')))}">
+        ]>
+        <bom xmlns="http://cyclonedx.org/schema/bom/${version}">
+          <components>
+            <component type="library">
+              <name>bar</name>
+              <version>1.337</version>
+              ${version === '1.0' ? '<modified>false</modified>' : ''}
+              <licenses>
+                <license>
+                  <id>&flag;</id>
+                </license>
+              </licenses>
+            </component>
+          </components>
+        </bom>`
+      const validationError = await validator.validate(input)
+      assert.doesNotMatch(
+        JSON.stringify(validationError),
+        /vaiquia2zoo3Im8ro9zahNg5mohwipouka2xieweed6ahChei3doox2fek3ise0lmohju3loh5oDu7eigh3jaeR2aiph2Voo/
+      )
+    })
   }))
 })