#1071: reject invalid XML element names in XML.toString (CWE-91)

[mechko]

Fixes #1071. Implements the throw-on-invalid-name approach discussed in #294 / #123, which also resolves the long-standing well-formedness question in #166 / #308.

Problem

XML.toString emits JSONObject keys verbatim as XML tag names. A key containing <, > or / breaks out of its element and injects arbitrary sibling structure:

{"a/><injected>evil</injected><a": ""}
    → <root><a/><injected>evil</injected><a/></root>

Values already go through escape(); keys/tagName do not.

Fix

Validate every key and tagName against the XML 1.0 (5th ed.) Name production before emitting; throw JSONException on mismatch.

• mustBeXmlName(String) — throws if the string is null/empty or contains a code point outside the Name production
• isXmlNameStart(int) / isXmlNameChar(int) — the two productions, code-point aware (handles supplementary planes)
• Called once for tagName at the top of the private toString overload, and once per key at the top of the key loop (the cDataTagName sentinel is skipped since it never becomes a tag)

<, >, /, whitespace, ", &, @ are all outside NameChar, so the injection vector is closed. Keys that are already valid XML Names are unaffected — mustBeXmlName is a no-op for them.

Behaviour change

XML.toString now throws for keys that are not valid XML Names, where previously it emitted malformed XML:

Key	Before	After
"a/><injected>…"	element injection	JSONException
"123foo"	<123foo> (invalid XML)	JSONException
"has space"	<has space> (invalid XML)	JSONException
"foo@bar"	<foo@bar> (invalid XML)	JSONException
"ns:name", "a-b.c_d", "élément"	unchanged	unchanged

The two existing shouldHandleIllegalJSONNodeNames tests already documented the old output as "invalid XML" / "possible bug"; they've been rewritten to assert the throw.

Verification

$ java -cp target/classes:. XmlRepro
Exception in thread "main" org.json.JSONException: 'a/><injected>evil</injected><a' is not a valid XML element name.
        at org.json.XML.mustBeXmlName(XML.java:259)
        at org.json.XML.toString(XML.java:1063)

mvn test: 791 run, 0 fail, 0 error, 6 skipped (pre-existing).

Tests

• XMLTest.shouldHandleIllegalJSONNodeNames / XMLConfigurationTest.shouldHandleIllegalJSONNodeNames — rewritten to assert JSONException for 123IllegalNode and Illegal@node
• XMLTest.toStringRejectsElementInjectionInKey — the XML.toString: unescaped keys allow XML element injection (CWE-91) — proposal to throw per #294 #1071 payload throws; invalid caller-supplied tagName throws
• XMLTest.toStringAcceptsValidXmlNames — hyphen/dot/underscore/colon, Latin-1 letters, and cDataTagName sentinel still serialise

🤖 Generated with Claude Code

[stleary]

@mechko Sorry for the long wait, will get to this before next week. While reviewing the PR, I found that strict mode is not filtering incoming text for valid JSON chars. I will fix this for strict mode, but keep the current behavior for non-strict mode.

[stleary]

@mechko Thanks for the PR. Can you please restrict this fix to the security issues mentioned in #1071. Enforcing well-formed tags could break backwards compatibility. Fixing the security issues also does this, but is justified because the fix prevents an injection attack.

• Only the chars < > & " ' / in tag names need to be caught
• Please remove the description text referring to well-formedness and XML.toString() need to handle invalid characters in nodeName #166 / XML.toString() returns invalid xml #308

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[mechko]

@stleary I adjusted the fix according to your comment, please let me know if you still want anything to be changed.