Skip to content

fix: upgrade xml2js to 0.6.2 (CVE-2023-0842) with security tests#615

Merged
jimmyandrade merged 3 commits into
masterfrom
snyk-fix-81b85fcd025c027ce3a4d5f3f035ae00
Jul 2, 2026
Merged

fix: upgrade xml2js to 0.6.2 (CVE-2023-0842) with security tests#615
jimmyandrade merged 3 commits into
masterfrom
snyk-fix-81b85fcd025c027ce3a4d5f3f035ae00

Conversation

@snyk-bot

@snyk-bot snyk-bot commented Apr 9, 2023

Copy link
Copy Markdown
Contributor

Summary

Proposed changes

This PR remediates prototype pollution in xml2js (CVE-2023-0842 / SNYK-JS-XML2JS-5414874) used by getGlyphsData to validate SVG input.

  • Pin xml2js@0.6.2 (exact version) and regenerate package-lock.json.
  • Add proto-element.svg fixture and unit tests in glyphsData.test.ts that document and enforce the security fix:
    • require xml2js >= 0.5.0 (defineProperty instead of obj[key] assignment);
    • document the null-prototype PropertyDescriptor pattern from the upstream fix;
    • assert __proto__ XML elements are stored as own properties without polluting Object.prototype;
    • assert getGlyphsData does not pollute when parsing malicious SVG and when rejecting malformed XML.

Note: The original Snyk commit only changed package.json to ^0.5.0 and did not update the lockfile. This branch merges current master, pins 0.6.2, and adds explicit security tests (141 tests pass).

Related issue

N/A

Dependencies added/removed (if applicable)

  • Update: xml2js 0.4.230.6.2 (direct dependency)

Testing

  • I have added or updated tests that prove my fix is effective or that my feature works (if applicable)
    • Unit test — glyphsData.test.ts prototype pollution hardening (describe("prototype pollution hardening (CVE-2023-0842 / SNYK-JS-XML2JS-5414874)"))
    • Unit test — existing svg xml validation via xml2js suite
    • Full suite — npm test (141 tests)

How to test

  1. Run npm test — all 141 tests should pass.
  2. Run npm auditxml2js should no longer be reported as vulnerable.
  3. Review src/standalone/glyphsData.test.ts — security tests document why the empty-file guard and xml2js >= 0.5.0 are required.

Test configuration

N/A


Checklist

  • I have added corresponding labels to this PR (like bug, enhancement...);
  • My commits follow the Conventional Commits 1.0 Guidelines;
  • My code follows the style guidelines of this project;
  • My commits follow the Conventional Commits 1.0 Guidelines;
  • I have performed a self-review of my own code;
  • I have mapped technical debts found on my changes;
  • I have made changes to the documentation (if applicable);
  • My changes generate no new warnings or errors;

@JacksonTian

Copy link
Copy Markdown

Any one process it?

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to remediate Snyk-reported prototype pollution in xml2js by upgrading the dependency from 0.4.23 to 0.5.0 in the project’s npm dependencies.

Changes:

  • Updates xml2js version range in package.json to ^0.5.0.
  • (Intended) updates package-lock.json to resolve xml2js to a non-vulnerable version, though the current lockfile content still pins xml2js@0.4.23.

Reviewed changes

Copilot reviewed 1 out of 1 changed files in this pull request and generated 1 comment.

File Description
package.json Bumps xml2js dependency range to ^0.5.0.
package-lock.json Should reflect the resolved upgrade; currently appears to still lock xml2js to 0.4.23, so the remediation may not take effect.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread package.json
jimmyandrade and others added 2 commits July 2, 2026 02:34
Pin xml2js@0.6.2 and regenerate package-lock.json to remediate
CVE-2023-0842 (SNYK-JS-XML2JS-5414874). Add explicit unit tests that
document the defineProperty/null-prototype descriptor guard and verify
getGlyphsData does not pollute Object.prototype when parsing SVG with
__proto__ elements.

Supersedes the incomplete Snyk bump to ^0.5.0 (lockfile was untouched).

Co-authored-by: Cursor <cursoragent@cursor.com>
@jimmyandrade jimmyandrade changed the title [Snyk] Security upgrade xml2js from 0.4.23 to 0.5.0 fix: upgrade xml2js to 0.6.2 (CVE-2023-0842) with security tests Jul 2, 2026
@jimmyandrade

Copy link
Copy Markdown
Collaborator

@JacksonTian Yes — picking this up now on this same PR.

The original Snyk commit only bumped package.json to ^0.5.0 and never updated package-lock.json, so npm ci would still install 0.4.23. I merged current master into this branch, pinned xml2js@0.6.2, regenerated the lockfile, and added explicit unit tests for CVE-2023-0842 / prototype pollution (including the null-prototype PropertyDescriptor pattern and __proto__ SVG fixtures).

npm test passes (141 tests) and npm audit no longer reports xml2js. Ready for review.

@jimmyandrade

Copy link
Copy Markdown
Collaborator

Updated this branch (same PR — no superseding PR needed):

  • Merged current master
  • Pinned xml2js@0.6.2 + regenerated package-lock.json
  • Added unit tests for CVE-2023-0842 / prototype pollution in glyphsData.test.ts

Copilot review note addressed: the original Snyk diff did not update the lockfile; this branch does.

@jimmyandrade jimmyandrade merged commit df6ceb4 into master Jul 2, 2026
2 checks passed
@jimmyandrade jimmyandrade deleted the snyk-fix-81b85fcd025c027ce3a4d5f3f035ae00 branch July 2, 2026 05:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants