Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Limit the XML nesting depth for CVE-2022-45688 #720

Merged
merged 5 commits into from Feb 6, 2023
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
14 changes: 11 additions & 3 deletions src/main/java/org/json/XML.java
Expand Up @@ -232,7 +232,7 @@ public static void noSpace(String string) throws JSONException {
* @return true if the close tag is processed.
* @throws JSONException
*/
private static boolean parse(XMLTokener x, JSONObject context, String name, XMLParserConfiguration config)
private static boolean parse(XMLTokener x, JSONObject context, String name, XMLParserConfiguration config, int currentNestingDepth)
throws JSONException {
char c;
int i;
Expand Down Expand Up @@ -402,7 +402,11 @@ private static boolean parse(XMLTokener x, JSONObject context, String name, XMLP

} else if (token == LT) {
// Nested element
if (parse(x, jsonObject, tagName, config)) {
if (currentNestingDepth == config.getMaxNestingDepth()) {
throw x.syntaxError("Maximum nesting depth of " + config.getMaxNestingDepth() + " reached");
}

if (parse(x, jsonObject, tagName, config, currentNestingDepth + 1)) {
if (config.getForceList().contains(tagName)) {
// Force the value to be an array
if (jsonObject.length() == 0) {
Expand Down Expand Up @@ -644,6 +648,10 @@ public static JSONObject toJSONObject(Reader reader, boolean keepStrings) throws
* All values are converted as strings, for 1, 01, 29.0 will not be coerced to
* numbers but will instead be the exact value as seen in the XML document.
*
* This method can parse documents with a maximum nesting depth of 256. If you
cleydyr marked this conversation as resolved.
Show resolved Hide resolved
* need to parse documents with a nesting depth greater than 256, you should use
*
*
* @param reader The XML source reader.
* @param config Configuration options for the parser
* @return A JSONObject containing the structured data from the XML string.
Expand All @@ -655,7 +663,7 @@ public static JSONObject toJSONObject(Reader reader, XMLParserConfiguration conf
while (x.more()) {
x.skipPast("<");
if(x.more()) {
parse(x, jo, null, config);
parse(x, jo, null, config, 0);
}
}
return jo;
Expand Down
41 changes: 41 additions & 0 deletions src/main/java/org/json/XMLParserConfiguration.java
Expand Up @@ -16,6 +16,12 @@
*/
@SuppressWarnings({""})
public class XMLParserConfiguration {
/**
* Used to indicate there's no defined limit to the maximum nesting depth when parsing a XML
* document to JSON.
*/
public static final int UNDEFINED_MAXIMUM_NESTING_DEPTH = -1;

/** Original Configuration of the XML Parser. */
public static final XMLParserConfiguration ORIGINAL
= new XMLParserConfiguration();
Expand Down Expand Up @@ -54,6 +60,12 @@ public class XMLParserConfiguration {
*/
private Set<String> forceList;

/**
* When parsing the XML into JSON, specifies the tags whose values should be converted
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment is incorrect (just a duplicate from the previous section) - suggested content:

The maximum nesting depth when parsing a XML document to JSON.

* to arrays
*/
private int maxNestingDepth = UNDEFINED_MAXIMUM_NESTING_DEPTH;

/**
* Default parser configuration. Does not keep strings (tries to implicitly convert
* values), and the CDATA Tag Name is "content".
Expand Down Expand Up @@ -297,4 +309,33 @@ public XMLParserConfiguration withForceList(final Set<String> forceList) {
newConfig.forceList = Collections.unmodifiableSet(cloneForceList);
return newConfig;
}

/**
* The maximum nesting depth that the parser will descend before throwing an exception
* when parsing the XML into JSON.
* @return the maximum nesting depth set for this configuration
*/
public int getMaxNestingDepth() {
return maxNestingDepth;
}

/**
* Defines the maximum nesting depth that the parser will descend before throwing an exception
* when parsing the XML into JSON. The default max nesting depth is undefined, which means the
* parser will go as deep as the maximum call stack size allows. Using any negative value as a
* parameter is equivalent to setting no limit to the nesting depth.
* @param maxNestingDepth the maximum nesting depth allowed to the XML parser
* @return The existing configuration will not be modified. A new configuration is returned.
*/
public XMLParserConfiguration withMaxNestingDepth(int maxNestingDepth) {
XMLParserConfiguration newConfig = this.clone();

if (maxNestingDepth > UNDEFINED_MAXIMUM_NESTING_DEPTH) {
newConfig.maxNestingDepth = maxNestingDepth;
} else {
newConfig.maxNestingDepth = UNDEFINED_MAXIMUM_NESTING_DEPTH;
}

return newConfig;
}
}
23 changes: 23 additions & 0 deletions src/test/java/org/json/junit/XMLConfigurationTest.java
Expand Up @@ -1051,6 +1051,29 @@ public void testEmptyTagForceList() {

Util.compareActualVsExpectedJsonObjects(jsonObject, expetedJsonObject);
}

@Test
public void testMaxNestingDepthIsSet() {
XMLParserConfiguration xmlParserConfiguration = XMLParserConfiguration.ORIGINAL;

assertEquals(xmlParserConfiguration.getMaxNestingDepth(), XMLParserConfiguration.UNDEFINED_MAXIMUM_NESTING_DEPTH);

xmlParserConfiguration = xmlParserConfiguration.withMaxNestingDepth(42);

assertEquals(xmlParserConfiguration.getMaxNestingDepth(), 42);

xmlParserConfiguration = xmlParserConfiguration.withMaxNestingDepth(0);

assertEquals(xmlParserConfiguration.getMaxNestingDepth(), 0);

xmlParserConfiguration = xmlParserConfiguration.withMaxNestingDepth(-31415926);

assertEquals(xmlParserConfiguration.getMaxNestingDepth(), XMLParserConfiguration.UNDEFINED_MAXIMUM_NESTING_DEPTH);

xmlParserConfiguration = xmlParserConfiguration.withMaxNestingDepth(Integer.MIN_VALUE);

assertEquals(xmlParserConfiguration.getMaxNestingDepth(), XMLParserConfiguration.UNDEFINED_MAXIMUM_NESTING_DEPTH);
}

/**
* Convenience method, given an input string and expected result,
Expand Down
59 changes: 59 additions & 0 deletions src/test/java/org/json/junit/XMLTest.java
Expand Up @@ -1247,6 +1247,65 @@ public void testIndentComplicatedJsonObjectWithArrayAndWithConfig(){
fail("file writer error: " +e.getMessage());
}
}

@Test
public void testMaxNestingDepthOf42IsRespected() {
final String wayTooLongMalformedXML = new String(new char[6000]).replace("\0", "<a>");

final int maxNestingDepth = 42;

try {
XML.toJSONObject(wayTooLongMalformedXML, XMLParserConfiguration.ORIGINAL.withMaxNestingDepth(maxNestingDepth));

fail("Expecting a JSONException");
} catch (JSONException e) {
assertTrue("Wrong throwable thrown: not expecting message <" + e.getMessage() + ">",
e.getMessage().startsWith("Maximum nesting depth of " + maxNestingDepth));
}
}

@Test
public void testMaxNestingDepthIsRespectedWithValidXML() {
final String perfectlyFineXML = "<Test>\n" +
cleydyr marked this conversation as resolved.
Show resolved Hide resolved
" <employee>\n" +
" <name>sonoo</name>\n" +
" <salary>56000</salary>\n" +
" <married>true</married>\n" +
" </employee>\n" +
"</Test>\n";

final int maxNestingDepth = 1;

try {
XML.toJSONObject(perfectlyFineXML, XMLParserConfiguration.ORIGINAL.withMaxNestingDepth(maxNestingDepth));

fail("Expecting a JSONException");
} catch (JSONException e) {
assertTrue("Wrong throwable thrown: not expecting message <" + e.getMessage() + ">",
e.getMessage().startsWith("Maximum nesting depth of " + maxNestingDepth));
}
}

@Test
public void testMaxNestingDepthWithValidFittingXML() {
final String perfectlyFineXML = "<Test>\n" +
" <employee>\n" +
" <name>sonoo</name>\n" +
" <salary>56000</salary>\n" +
" <married>true</married>\n" +
" </employee>\n" +
"</Test>\n";

final int maxNestingDepth = 3;

try {
XML.toJSONObject(perfectlyFineXML, XMLParserConfiguration.ORIGINAL.withMaxNestingDepth(maxNestingDepth));
} catch (JSONException e) {
e.printStackTrace();
fail("XML document should be parsed as its maximum depth fits the maxNestingDepth " +
"parameter of the XMLParserConfiguration used");
}
}
cleydyr marked this conversation as resolved.
Show resolved Hide resolved
}


Expand Down