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]: #1061 [#1064]: #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 <[email protected]>
CycloneDX · May 8, 2024 · 5bd28e7 · 5bd28e7
1 parent e7bc72e
commit 5bd28e7
Show file tree

Hide file tree

Showing 4 changed files with 47 additions and 1 deletion.
diff --git 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
@@ -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
@@ -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
@@ -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/
+      )
+    })
   }))
 })