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

Fix bug Improper handling of exceptional conditions Newtonsoft #2465

Closed
wants to merge 2 commits into from

Conversation

functionofpwnosec
Copy link

JsonConvert.DeserializeObject vulnerable to Insecure Defaults due to improper handling of expressions with high nesting level that lead to StackOverFlow exception or high CPU and RAM usage. Deserializing methods (like JsonConvert.DeserializeObject) will process the input that results in burning the CPU, allocating memory, and consuming a thread of execution. Quite high nesting level (>10kk, or 9.5MB of {a:{a:{... input) is needed to achieve the latency over 10 seconds, depending on the hardware.

To mitigate the issue one either need to update Newtonsoft.Json to 13.0.1 or set MaxDepth parameter in the JsonSerializerSettings. This can be done globally with the following statement. After that the parsing of the nested input will fail fast with Newtonsoft.Json.JsonReaderException:

//Create a string representation of an highly nested object (JSON serialized)
int nRep = 25000;
string json = string.Concat(Enumerable.Repeat("{a:", nRep)) + "1" +
 string.Concat(Enumerable.Repeat("}", nRep));

//Parse this object (leads to high CPU/RAM consumption)
var parsedJson = JsonConvert.DeserializeObject(json);

// Methods below all throw stack overflow with nRep around 20k and higher
// string a = parsedJson.ToString();
// string b = JsonConvert.SerializeObject(parsedJson);

var keyValues = JsonConvert.DeserializeObject<IDictionary<string, string>>(httpResponseBody);

WeaknessCWE-755

Copy link

cla-checker-service bot commented Oct 11, 2024

💚 CLA has been signed

Copy link

👋 @functionofpwnosec Thanks a lot for your contribution!

It may take some time before we review a PR, so even if you don’t see activity for some time, it does not mean that we have forgotten about it.

Every once in a while we go through a process of prioritization, after which we are focussing on the tasks that were planned for the upcoming milestone. The prioritization status is typically reflected through the PR labels. It could be pending triage, a candidate for a future milestone, or have a target milestone set to it.

@Mpdreamz
Copy link
Member

Thanks for the PR @functionofpwnsec 👍

This CVE is not in code we ship but the PR is most welcome regardless.

I addressed this as part of #2467 so will close this PR in favour of the other.

@Mpdreamz Mpdreamz closed this Oct 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants