From 43cc75216e7c08248f72b69e23a5f2b4c410af8f Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:04:34 +0200 Subject: [PATCH 1/8] test: reproduce reported security issues for #1061 Signed-off-by: Jan Kowalleck --- .github/workflows/nodejs.yml | 4 ++ .../Validation.XmlValidator.test.js | 42 +++++++++++++++++++ 2 files changed, 46 insertions(+) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index aaf761cdc..b2f3d41e5 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -119,6 +119,10 @@ jobs: - ubuntu-latest - macos-13 # macos-latest has issues with node14 - windows-latest + includes: + - # + node-version: 20 + os: ubuntu-latest timeout-minutes: 10 steps: - name: Checkout diff --git a/tests/integration/Validation.XmlValidator.test.js b/tests/integration/Validation.XmlValidator.test.js index 17a273aa8..7b7ef982e 100644 --- a/tests/integration/Validation.XmlValidator.test.js +++ b/tests/integration/Validation.XmlValidator.test.js @@ -99,5 +99,47 @@ describe('Validation.XmlValidator', () => { const validationError = await validator.validate(input) assert.strictEqual(validationError, null) }) + + it('is not vulnerable to advisories/GHSA-mjr4-7xg5-pfvh', async () => { + /* report: + see https://github.com/advisories/GHSA-mjr4-7xg5-pfvh + see https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 + */ + const validator = new XmlValidator(version) + /* POC payload: + see https://research.jfrog.com/vulnerabilities/libxmljs2-attrs-type-confusion-rce-jfsa-2024-001034097/#poc + */ + const input = ` + +]> +&writer; +`; + const validationError = await validator.validate(input) + // expected to not crash ... + assert.strictEqual(validationError, null) + }) + + it('is not vulnerable to advisories/GHSA-78h3-pg4x-j8cv', async () => { + /* report: + see https://github.com/advisories/GHSA-78h3-pg4x-j8cv + see https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 + */ + const validator = new XmlValidator(version) + /* POC payload: + see https://research.jfrog.com/vulnerabilities/libxmljs2-namespaces-type-confusion-rce-jfsa-2024-001034098/#poc + */ + const input = ` + +]> +&writer; +`; + const validationError = await validator.validate(input) + // expected to not crash ... + assert.strictEqual(validationError, null) + }) })) }) From 56e296631d7728956cbf105934b60b4bfd307de9 Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:18:59 +0200 Subject: [PATCH 2/8] fix Signed-off-by: Jan Kowalleck --- src/validation/xmlValidator.node.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/validation/xmlValidator.node.ts b/src/validation/xmlValidator.node.ts index 667657ad0..08601be69 100644 --- a/src/validation/xmlValidator.node.ts +++ b/src/validation/xmlValidator.node.ts @@ -48,7 +48,8 @@ async function getParser (): Promise { const xmlParseOptions: Readonly = Object.freeze({ nonet: true, - compact: true + compact: true, + noent: true // prevent https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 }) export class XmlValidator extends BaseValidator { From 709268a6ad9980497146f9809e32d0bdc529d32b Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:20:59 +0200 Subject: [PATCH 3/8] tests Signed-off-by: Jan Kowalleck --- .../Validation.XmlValidator.test.js | 38 ++++++++++++------- 1 file changed, 24 insertions(+), 14 deletions(-) diff --git a/tests/integration/Validation.XmlValidator.test.js b/tests/integration/Validation.XmlValidator.test.js index 7b7ef982e..3818e1612 100644 --- a/tests/integration/Validation.XmlValidator.test.js +++ b/tests/integration/Validation.XmlValidator.test.js @@ -110,14 +110,19 @@ describe('Validation.XmlValidator', () => { see https://research.jfrog.com/vulnerabilities/libxmljs2-attrs-type-confusion-rce-jfsa-2024-001034097/#poc */ const input = ` - -]> -&writer; -`; + + ]> + + + + &writer; + 1.337 + + + ` const validationError = await validator.validate(input) - // expected to not crash ... assert.strictEqual(validationError, null) }) @@ -131,14 +136,19 @@ describe('Validation.XmlValidator', () => { see https://research.jfrog.com/vulnerabilities/libxmljs2-namespaces-type-confusion-rce-jfsa-2024-001034098/#poc */ const input = ` - -]> -&writer; -`; + + ]> + + + + &writer; + 1.337 + + + ` const validationError = await validator.validate(input) - // expected to not crash ... assert.strictEqual(validationError, null) }) })) From dc137524e84f60b0ed05f68126faa4dec9897889 Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:31:08 +0200 Subject: [PATCH 4/8] tidy Signed-off-by: Jan Kowalleck --- tests/integration/Validation.XmlValidator.test.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/integration/Validation.XmlValidator.test.js b/tests/integration/Validation.XmlValidator.test.js index 3818e1612..3ecc5ef7b 100644 --- a/tests/integration/Validation.XmlValidator.test.js +++ b/tests/integration/Validation.XmlValidator.test.js @@ -100,7 +100,7 @@ describe('Validation.XmlValidator', () => { assert.strictEqual(validationError, null) }) - it('is not vulnerable to advisories/GHSA-mjr4-7xg5-pfvh', async () => { + it('is not vulnerable to advisories/GHSA-mjr4-7xg5-pfvh', async () => { /* report: see https://github.com/advisories/GHSA-mjr4-7xg5-pfvh see https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 @@ -126,7 +126,7 @@ describe('Validation.XmlValidator', () => { assert.strictEqual(validationError, null) }) - it('is not vulnerable to advisories/GHSA-78h3-pg4x-j8cv', async () => { + it('is not vulnerable to advisories/GHSA-78h3-pg4x-j8cv', async () => { /* report: see https://github.com/advisories/GHSA-78h3-pg4x-j8cv see https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 @@ -138,7 +138,7 @@ describe('Validation.XmlValidator', () => { const input = ` + ]> From cc9c21458fa75439039e4f37ee89141e3310d160 Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:34:53 +0200 Subject: [PATCH 5/8] docs Signed-off-by: Jan Kowalleck --- HISTORY.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/HISTORY.md b/HISTORY.md index 67e42f946..798331ed6 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -6,6 +6,13 @@ All notable changes to this project will be documented in this file. +* Changed + * XmlValidation no longer supports XML external entities (via [#1063]; concerns [#1061]) + This is considered a security measure to prevent XML external entity (XXE) injection. + +[#1061]: https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 +[#1063]: https://github.com/CycloneDX/cyclonedx-javascript-library/pull/1063 + ## 6.6.1 -- 2024-05-06 * Fixed From 91f6d27f1ea9b4546e42908fbbfbc030d6acd0d9 Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:36:12 +0200 Subject: [PATCH 6/8] docs Signed-off-by: Jan Kowalleck --- HISTORY.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/HISTORY.md b/HISTORY.md index 798331ed6..30ddf7ea1 100644 --- a/HISTORY.md +++ b/HISTORY.md @@ -7,7 +7,7 @@ All notable changes to this project will be documented in this file. * Changed - * XmlValidation no longer supports XML external entities (via [#1063]; concerns [#1061]) + * The provided XML validation capabilities no longer supports external entities (via [#1063]; concerns [#1061]) This is considered a security measure to prevent XML external entity (XXE) injection. [#1061]: https://github.com/CycloneDX/cyclonedx-javascript-library/issues/1061 From 60cca6cbb0c7940bb5dcb336f142d38af8c0aaa0 Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:36:54 +0200 Subject: [PATCH 7/8] clean Signed-off-by: Jan Kowalleck --- .github/workflows/nodejs.yml | 4 ---- 1 file changed, 4 deletions(-) diff --git a/.github/workflows/nodejs.yml b/.github/workflows/nodejs.yml index b2f3d41e5..aaf761cdc 100644 --- a/.github/workflows/nodejs.yml +++ b/.github/workflows/nodejs.yml @@ -119,10 +119,6 @@ jobs: - ubuntu-latest - macos-13 # macos-latest has issues with node14 - windows-latest - includes: - - # - node-version: 20 - os: ubuntu-latest timeout-minutes: 10 steps: - name: Checkout From 96ae0656e0e5bd40817c1ddaaecf357791b138fd Mon Sep 17 00:00:00 2001 From: Jan Kowalleck Date: Tue, 7 May 2024 14:44:46 +0200 Subject: [PATCH 8/8] test Signed-off-by: Jan Kowalleck --- tests/integration/Validation.XmlValidator.test.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/tests/integration/Validation.XmlValidator.test.js b/tests/integration/Validation.XmlValidator.test.js index 3ecc5ef7b..bbdf6bd9e 100644 --- a/tests/integration/Validation.XmlValidator.test.js +++ b/tests/integration/Validation.XmlValidator.test.js @@ -119,6 +119,7 @@ describe('Validation.XmlValidator', () => { &writer; 1.337 + ${version === '1.0' ? 'false' : ''} ` @@ -145,6 +146,7 @@ describe('Validation.XmlValidator', () => { &writer; 1.337 + ${version === '1.0' ? 'false' : ''} `