XML.toString: unescaped keys allow XML element injection (CWE-91) — proposal to throw per #294

[mechko]

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:

1. Impact is injection, not just malformed output (PoC below).
2. 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.

[stleary]

@mechko Interesting, thanks for opening this issue, please go ahead and submit a PR.

[mechko]

Opened #1072 against master with the throw-on-invalid-Name approach + tests as discussed above.