Repository navigation
XML.toString: unescaped keys allow XML element injection (CWE-91) — proposal to throw per #294 #1071
Description
Summary
XML.toString(JSONObject) emits JSONObject keys verbatim as XML element
names. A key containing <, > or / breaks out of its own tag and
injects arbitrary sibling elements into the output. When the JSONObject
is built from untrusted JSON (new JSONObject(String) — the documented
JSON→XML flow), that is attacker-controlled structure in the emitted XML.
Prior art
This has come up before as a well-formedness question — #123, #138,
#166, #294, #308 — with the standing guidance being "caller must
pre-process keys." I'm re-raising because:
- Impact is injection, not just malformed output (PoC below).
- In XML.toString does not handle the case where JSON keys are not valid XML element names. #294 you wrote
"No objection to someone doing this now" re: throwing on invalid
input — this issue is that "someone."
Reproduction (master @ 1795e8c / release 20260719)
import org.json.*;
public class Repro {
public static void main(String[] a) {
JSONObject jo = new JSONObject(
"{\"a/><injected>evil</injected><a\":\"\"}");
String xml = XML.toString(jo, "root");
System.out.println("XML output: " + xml);
System.out.println("Re-parsed: " + XML.toJSONObject(xml).toString(2));
}
}XML output: <root><a/><injected>evil</injected><a/></root>
Re-parsed: {"root": {
"a": [
"",
""
],
"injected": "evil"
}}
A single JSON key produces an attacker-chosen <injected> element that
survives a round-trip through XML.toJSONObject. Downstream impact
depends on the XML consumer (config/SOAP smuggling, stored XSS if later
rendered, XXE pivot if the consumer resolves an injected <!DOCTYPE>).
Affected code
XML.toString(Object, String, XMLParserConfiguration, int, int) in
src/main/java/org/json/XML.java: tagName at lines 977 / 1066 /
1100 / 1102–1103 and key at lines 1023 / 1027 / 1037 / 1040 / 1048
are appended between < / > without validation. Values already go
through escape(); keys do not.
Proposed fix — throw on invalid XML Name (per #294 / #123)
Reject any key or tagName that is not a valid XML Name:
private static void mustBeXmlName(String name) throws JSONException {
if (name == null || name.isEmpty()
|| !isXmlNameStart(name.charAt(0))) {
throw new JSONException("'" + name + "' is not a valid XML element name");
}
for (int i = 1; i < name.length(); i++) {
if (!isXmlNameChar(name.charAt(i))) {
throw new JSONException("'" + name + "' is not a valid XML element name");
}
}
}called once for tagName at method entry and once per key at the top
of the key loop. <, >, /, space, ", & are all outside the XML
Name production, so the injection vector is closed. Keys that are
already valid element names are unaffected.
This is the option you indicated was acceptable in #294 ("changing
users' content is not an option … no objection to [throwing]"). If
you'd instead prefer the JSONML approach (XML.escape(tagName),
JSONML.java:557–558), happy to do that — but going with your stated
preference by default.
I have a patch + unit test ready and can open a PR against master.
CWE-91. Affects all releases ≤ 20260719.
Activity
stleary commentedon Jul 21, 2026
@mechko Interesting, thanks for opening this issue, please go ahead and submit a PR.
mechko commentedon Jul 23, 2026
Opened #1072 against
masterwith the throw-on-invalid-Name approach + tests as discussed above.stleary#1071: extract inRange helper to satisfy Sonar S3776
stleary#1071: narrow tag-name check to XML metachars only
Merge pull request #1072 from mechko/1071-xml-tostring-name-validation
stleary#1071: reject invalid XML element names in XML.toString (CWE-91)