Skip to content

fast-xml-parser regex vulnerability patch could be improved from a safety perspective

Low severity GitHub Reviewed Published Jun 13, 2023 in NaturalIntelligence/fast-xml-parser • Updated Nov 29, 2023

Package

npm fast-xml-parser (npm)

Affected versions

= 4.2.4

Patched versions

4.2.5

Description

Summary

This is a comment on GHSA-6w63-h3fj-q4vw and the patches fixing it.

Details

The code which validates a name calls the validator:
https://github.com/NaturalIntelligence/fast-xml-parser/blob/ecf6016f9b48aec1a921e673158be0773d07283e/src/xmlparser/DocTypeReader.js#L145-L153
This checks for the presence of an invalid character. Such an approach is always risky, as it is so easy to forget to include an invalid character in the list. A safer approach is to validate entity names against the XML specification: https://www.w3.org/TR/xml11/#sec-common-syn - an ENTITY name is a Name:

[4]   NameStartChar ::= ":" | [A-Z] | "_" | [a-z] | [#xC0-#xD6] | [#xD8-#xF6] | [#xF8-#x2FF] | [#x370-#x37D] |
                        [#x37F-#x1FFF] | [#x200C-#x200D] | [#x2070-#x218F] | [#x2C00-#x2FEF] | [#x3001-#xD7FF] |
                        [#xF900-#xFDCF] | [#xFDF0-#xFFFD] | [#x10000-#xEFFFF]
[4a]  NameChar ::= NameStartChar | "-" | "." | [0-9] | #xB7 | [#x0300-#x036F] | [#x203F-#x2040]
[5]   Name ::= NameStartChar (NameChar)*

so the safest way to validate an entity name is to build a regex to represent this expression and check whether the name given matches the regex. (Something along the lines of /^[name start char class][name char class]*$/.) There's probably a nice way to simplify the explicit list rather than typing it out verbatim using Unicode character properties, but I don't know enough to do so.

References

Published to the GitHub Advisory Database Jun 15, 2023
Reviewed Jun 15, 2023
Last updated Nov 29, 2023

Severity

Low

Weaknesses

No CWEs

CVE ID

No known CVE

GHSA ID

GHSA-gpv5-7x3g-ghjv

Credits

Loading Checking history
See something to contribute? Suggest improvements for this vulnerability.