Skip to content

Commit

Permalink
Merge pull request #720 from cleydyr/issue-708
Browse files Browse the repository at this point in the history
Limit the XML nesting depth for CVE-2022-45688
  • Loading branch information
stleary authored Feb 6, 2023
2 parents 5920eca + 448e204 commit 401495a
Show file tree
Hide file tree
Showing 5 changed files with 135 additions and 4 deletions.
10 changes: 7 additions & 3 deletions src/main/java/org/json/XML.java
Original file line number Diff line number Diff line change
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 @@ -655,7 +659,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
46 changes: 46 additions & 0 deletions src/main/java/org/json/XMLParserConfiguration.java
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,17 @@
*/
@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;

/**
* The default maximum nesting depth when parsing a XML document to JSON.
*/
public static final int DEFAULT_MAXIMUM_NESTING_DEPTH = 512;

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

/**
* When parsing the XML into JSON, specifies the tags whose values should be converted
* to arrays
*/
private int maxNestingDepth = DEFAULT_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 +314,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 512, 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;
}
}
1 change: 0 additions & 1 deletion src/test/java/org/json/junit/JSONArrayTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,6 @@

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertFalse;
import static org.junit.Assert.assertNotEquals;
import static org.junit.Assert.assertNotNull;
import static org.junit.Assert.assertNull;
import static org.junit.Assert.assertTrue;
Expand Down
23 changes: 23 additions & 0 deletions src/test/java/org/json/junit/XMLConfigurationTest.java
Original file line number Diff line number Diff line change
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.DEFAULT_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
Original file line number Diff line number Diff line change
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" +
" <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");
}
}
}


Expand Down

0 comments on commit 401495a

Please sign in to comment.