There are multiple issues where FXP is being too lenient in parsing XML. This is disappointing, because one of the features of XML when it came out in the late 1990s is that it requires unequivocally that errors must bring the parser to a full stop. That way the user could be fully confident of retrieving the correct data. Many modern parsers unfortunately seem to take the approach that leniency is a virtue.
Leniency would be helpful for parsing crummy, hand-written HTML, and adding a leniency mode for that is useful. But parsing XML in a lenient way only lowers confidence in the data and in fact introduces security issues, as laid out below. I'm combining these leniency issues into one ticket because there are a number of them, and they are related. (Note that saxes doesn't have many of these issues; apparently it was written to be a true XML parser.)
This investigation (and bug report) was performed using Claude Opus 4.8. (Double-check the actual XML specification wording as it was generated from the LLM's training.)
Overview
This ticket bundles several independent places where FXP accepts — or silently mis-parses — input that XML 1.0/1.1 defines as not well-formed. They are combined into one report because they share a single root characteristic (FXP is a lenient, largely non-validating parser: it enforces tag nesting but not most other well-formedness constraints) and a common security profile (below). Each row is independently actionable and independently fixable; the table gives the location of each, and the sections after it add detail and a suggested fix where the fix isn't obvious.
Scope note: this is not about the values returned for well-formed input — that correctness is covered by two separate fidelity tickets (#11, #12), cross-referenced in the table for context. This ticket is strictly about malformed input being accepted or mis-handled instead of rejected.
Potential security impact
Leniency here is not only a correctness matter. Because a lenient parser can quietly disagree with a stricter reader, or let through bytes a stricter reader would reject, it opens two well-known attack classes when XML from an untrusted source is parsed with FXP.
-
Parser differential / content smuggling. When FXP silently accepts or reinterprets a construct that another reader sees differently — a duplicate attribute resolving last-wins, an unquoted value dropped and re-read as a different attribute, a second root element, or ]]>/</-- in a forbidden position — the two readers no longer agree on what the document says, so whatever the other reader checked for can be bypassed. XML forbids duplicate attributes for exactly this reason; FXP doesn't enforce it and silently keeps the last one. An incoming command reviewed by a person, or by a check that keys on the first action it finds, and then executed by a backend using FXP, can carry two different instructions at once:
<command action="read" action="delete" target="/prod-db"/>
The review sees the first, harmless read and approves it; FXP parses the identical bytes and executes delete. The unquoted-value case is the same weakness from another angle — <command action=read cmd="run"/> — where FXP drops action entirely and reparses read as a stray attribute, so the command reaching the backend isn't the one any strict reader saw.
-
Injection via unvalidated characters. FXP performs no Char-production validation, so control characters that XML forbids in content pass straight through into parsed values and on to whatever consumes them. A value in an otherwise ordinary field can carry an ANSI escape sequence that a conformant parser would reject but FXP passes through verbatim:
Reject this request.\x1B[2K\x1B[1A ✓ request approved — signature verified
Printed to a terminal or log, \x1B[2K erases the current line and \x1B[1A moves the cursor up, so the escape codes execute instead of displaying and paint a fake "approved" line over the real one. A NUL byte (\x00) works the same way in another form, truncating a value mid-write so the remainder of a blocked string slips past a downstream check that stops reading at the null byte.
Neither class is a crash or an RCE. The risk is quieter: a caller assumes "it parsed, so it was valid XML" and inherits these as latent weaknesses that a strict parser would have caught simply by refusing the document.
Environment
@nodable/flexible-xml-parser@1.10.1 (the constraints below are enforced by no output builder, so they reproduce through @nodable/sax@1.1.0 and compact-builder alike).
Summary
Strictness gaps (this ticket):
| XML constraint |
FXP behavior |
saxes behavior |
Location in FXP |
Illegal characters — Char production (§2.2): control chars (0x00–0x08, 0x0B, 0x0C, 0x0E–0x1F) are forbidden |
Passed through unvalidated into text/attr/CDATA values |
Fatal ("disallowed character") |
InputSource read path (readCh/readStr return raw buffer[i]); no isChar exists in util.js |
| Unique Att Spec (WFC): an element may not repeat an attribute name |
No check — last value wins; via SAX, onAttribute fires twice |
Fatal ("duplicate attribute") |
AttributeProcessor.js → collectRawAttributes() |
| Attribute values must be quoted (§3.1) |
Unquoted value is silently dropped and its text re-scanned as a boolean attribute |
Fatal ("unquoted attribute value") |
AttributeProcessor.js → parseAttributes() |
No literal < in an attribute value (§3.1, AttValue) |
Accepted verbatim |
Fatal ("disallowed character") |
AttributeProcessor.js → parseAttributes(); scanTagExpEnd.js |
No -- within a comment (§2.5) |
Accepted |
Fatal ("malformed comment") |
XmlSpecialTagsReader.js → readComment() |
No ]]> in character data (§2.4) |
Accepted as text |
Fatal ("]]> disallowed in char data") |
Xml2JsParser.js → parseXml() text loop |
| One root element; no markup/second root after it (§2.1) |
Second root element accepted; empty/rootless document accepted (only non-whitespace trailing text is rejected) |
Fatal ("only one root" / "must contain a root element") |
Xml2JsParser.js → parseXml() / finalizeXml() |
Whitespace set — S (§2.3) is exactly #x20, #x9, #xD, #xA |
Form-feed (#xC) is also treated as whitespace |
Correct set |
util.js → isSpace() / isSpaceCode() |
| XML declaration validity & position (§2.8) |
version/encoding/standalone values unvalidated; pseudo-attribute order and "must be first" position unenforced |
Validated (formats, order, position) |
XmlSpecialTagsReader.js → readPiTag() (carries //TODO: verify it is very first tag else error) |
Fidelity issues (filed separately — listed only for a complete picture):
| Issue |
XML rule |
FXP behavior |
saxes behavior |
| #11 |
§2.11 end-of-line normalization (CR/CRLF → LF, incl. XML 1.1 NEL/LS) |
Not applied in text/CDATA |
Applied |
| #12 |
§3.3.3 attribute-value normalization |
Literal TAB not mapped to space |
Applied |
Details & suggested fixes
Illegal characters not validated (Char production)
The character-reading path (StringSource/FeedableSource readCh/readStr) returns raw buffer characters, and no reader validates them; util.js has no isChar counterpart to saxes's getCode10/getCode11. A document containing, say, a raw NUL or ESC inside element text is parsed and the character surfaces intact in the reported value. Fix: validate each code point against the Char production in the read path (an isChar(code) gate, XML-version-dependent as saxes does). This is the most performance-sensitive fix, so it likely belongs in the hot read loop rather than per-reader.
Duplicate attributes not rejected
collectRawAttributes() does tagExp.rawAttributes[m.name] = attrVal (later duplicate overwrites earlier) and unconditionally parsedAttrs.push(...) (so the SAX onAttribute fires once per occurrence). Nothing detects the repeat. Fix: track seen names in collectRawAttributes() (a Set, or an in check against rawAttributes) and throw a ParseError on the second occurrence.
Unquoted attribute values silently drop data
In parseAttributes(), after consuming = and surrounding whitespace, the value is read only inside if (quote === 34 || quote === 39) { … results.push(…) }. If the next character is not a quote, that block is skipped, i is not advanced, and nothing is pushed — so on the next loop iteration the value's text is read as a fresh attribute name. Net effect for <e a=b>: attribute a is discarded and b becomes a boolean attribute — silent data loss, not an error. Fix: add the missing branch — an unquoted value is not well-formed, so throw a ParseError when the post-= character is not " or '.
Root-element constraints not enforced
After the root closes, currentTagDetail returns to the synthetic root and parseXml() keeps looping; a second top-level element is pushed with no guard. finalizeXml() only flags non-whitespace trailing text (hasTrailingText), so a second complete root element passes, and an empty input (no root at all) also passes. Fix: carry a "root seen / root closed" flag; in readOpeningTag() reject opening a second element once the root has closed, and in finalizeXml() reject a document in which no root element was ever seen.
Simpler fixes (one-line class)
< in attribute value — parseAttributes(): while scanning a quoted value, reject a < (code 60) before the closing quote.-- in comment — readComment(): readUpto("-->") can't see interior --; scan for -- and reject unless immediately followed by > (mirrors saxes).]]> in character data — parseXml() text loop: detect the ]]> sequence in the run and throw, rather than only breaking on <.- Form-feed as whitespace —
util.js: remove \f / code 12 from isSpace() and isSpaceCode(); XML S is only space, \t, \r, \n.
- XML declaration —
readPiTag(): validate version against 1\.\d+, standalone against yes|no, enforce version→encoding→standalone order, and resolve the existing //TODO by rejecting a declaration that is not at the very start of the document.
There are multiple issues where FXP is being too lenient in parsing XML. This is disappointing, because one of the features of XML when it came out in the late 1990s is that it requires unequivocally that errors must bring the parser to a full stop. That way the user could be fully confident of retrieving the correct data. Many modern parsers unfortunately seem to take the approach that leniency is a virtue.
Leniency would be helpful for parsing crummy, hand-written HTML, and adding a leniency mode for that is useful. But parsing XML in a lenient way only lowers confidence in the data and in fact introduces security issues, as laid out below. I'm combining these leniency issues into one ticket because there are a number of them, and they are related. (Note that
saxesdoesn't have many of these issues; apparently it was written to be a true XML parser.)This investigation (and bug report) was performed using Claude Opus 4.8. (Double-check the actual XML specification wording as it was generated from the LLM's training.)
Overview
This ticket bundles several independent places where FXP accepts — or silently mis-parses — input that XML 1.0/1.1 defines as not well-formed. They are combined into one report because they share a single root characteristic (FXP is a lenient, largely non-validating parser: it enforces tag nesting but not most other well-formedness constraints) and a common security profile (below). Each row is independently actionable and independently fixable; the table gives the location of each, and the sections after it add detail and a suggested fix where the fix isn't obvious.
Scope note: this is not about the values returned for well-formed input — that correctness is covered by two separate fidelity tickets (#11, #12), cross-referenced in the table for context. This ticket is strictly about malformed input being accepted or mis-handled instead of rejected.
Potential security impact
Leniency here is not only a correctness matter. Because a lenient parser can quietly disagree with a stricter reader, or let through bytes a stricter reader would reject, it opens two well-known attack classes when XML from an untrusted source is parsed with FXP.
Parser differential / content smuggling. When FXP silently accepts or reinterprets a construct that another reader sees differently — a duplicate attribute resolving last-wins, an unquoted value dropped and re-read as a different attribute, a second root element, or
]]>/</--in a forbidden position — the two readers no longer agree on what the document says, so whatever the other reader checked for can be bypassed. XML forbids duplicate attributes for exactly this reason; FXP doesn't enforce it and silently keeps the last one. An incoming command reviewed by a person, or by a check that keys on the firstactionit finds, and then executed by a backend using FXP, can carry two different instructions at once:The review sees the first, harmless
readand approves it; FXP parses the identical bytes and executesdelete. The unquoted-value case is the same weakness from another angle —<command action=read cmd="run"/>— where FXP dropsactionentirely and reparsesreadas a stray attribute, so the command reaching the backend isn't the one any strict reader saw.Injection via unvalidated characters. FXP performs no
Char-production validation, so control characters that XML forbids in content pass straight through into parsed values and on to whatever consumes them. A value in an otherwise ordinary field can carry an ANSI escape sequence that a conformant parser would reject but FXP passes through verbatim:Printed to a terminal or log,
\x1B[2Kerases the current line and\x1B[1Amoves the cursor up, so the escape codes execute instead of displaying and paint a fake "approved" line over the real one. ANULbyte (\x00) works the same way in another form, truncating a value mid-write so the remainder of a blocked string slips past a downstream check that stops reading at the null byte.Neither class is a crash or an RCE. The risk is quieter: a caller assumes "it parsed, so it was valid XML" and inherits these as latent weaknesses that a strict parser would have caught simply by refusing the document.
Environment
@nodable/flexible-xml-parser@1.10.1(the constraints below are enforced by no output builder, so they reproduce through@nodable/sax@1.1.0andcompact-builderalike).Summary
Strictness gaps (this ticket):
saxesbehaviorCharproduction (§2.2): control chars (0x00–0x08, 0x0B, 0x0C, 0x0E–0x1F) are forbiddenInputSourceread path (readCh/readStrreturn rawbuffer[i]); noisCharexists inutil.jsonAttributefires twiceAttributeProcessor.js→collectRawAttributes()AttributeProcessor.js→parseAttributes()<in an attribute value (§3.1,AttValue)AttributeProcessor.js→parseAttributes();scanTagExpEnd.js--within a comment (§2.5)XmlSpecialTagsReader.js→readComment()]]>in character data (§2.4)Xml2JsParser.js→parseXml()text loopXml2JsParser.js→parseXml()/finalizeXml()S(§2.3) is exactly #x20, #x9, #xD, #xAutil.js→isSpace()/isSpaceCode()XmlSpecialTagsReader.js→readPiTag()(carries//TODO: verify it is very first tag else error)Fidelity issues (filed separately — listed only for a complete picture):
saxesbehaviorDetails & suggested fixes
Illegal characters not validated (
Charproduction)The character-reading path (
StringSource/FeedableSourcereadCh/readStr) returns raw buffer characters, and no reader validates them;util.jshas noisCharcounterpart tosaxes'sgetCode10/getCode11. A document containing, say, a rawNULorESCinside element text is parsed and the character surfaces intact in the reported value. Fix: validate each code point against theCharproduction in the read path (anisChar(code)gate, XML-version-dependent assaxesdoes). This is the most performance-sensitive fix, so it likely belongs in the hot read loop rather than per-reader.Duplicate attributes not rejected
collectRawAttributes()doestagExp.rawAttributes[m.name] = attrVal(later duplicate overwrites earlier) and unconditionallyparsedAttrs.push(...)(so the SAXonAttributefires once per occurrence). Nothing detects the repeat. Fix: track seen names incollectRawAttributes()(aSet, or anincheck againstrawAttributes) and throw aParseErroron the second occurrence.Unquoted attribute values silently drop data
In
parseAttributes(), after consuming=and surrounding whitespace, the value is read only insideif (quote === 34 || quote === 39) { … results.push(…) }. If the next character is not a quote, that block is skipped,iis not advanced, and nothing is pushed — so on the next loop iteration the value's text is read as a fresh attribute name. Net effect for<e a=b>: attributeais discarded andbbecomes a boolean attribute — silent data loss, not an error. Fix: add the missing branch — an unquoted value is not well-formed, so throw aParseErrorwhen the post-=character is not"or'.Root-element constraints not enforced
After the root closes,
currentTagDetailreturns to the synthetic root andparseXml()keeps looping; a second top-level element is pushed with no guard.finalizeXml()only flags non-whitespace trailing text (hasTrailingText), so a second complete root element passes, and an empty input (no root at all) also passes. Fix: carry a "root seen / root closed" flag; inreadOpeningTag()reject opening a second element once the root has closed, and infinalizeXml()reject a document in which no root element was ever seen.Simpler fixes (one-line class)
<in attribute value —parseAttributes(): while scanning a quoted value, reject a<(code 60) before the closing quote.--in comment —readComment():readUpto("-->")can't see interior--; scan for--and reject unless immediately followed by>(mirrorssaxes).]]>in character data —parseXml()text loop: detect the]]>sequence in the run and throw, rather than only breaking on<.util.js: remove\f/ code12fromisSpace()andisSpaceCode(); XMLSis only space,\t,\r,\n.readPiTag(): validateversionagainst1\.\d+,standaloneagainstyes|no, enforce version→encoding→standalone order, and resolve the existing//TODOby rejecting a declaration that is not at the very start of the document.