Skip to content

Fix bug Improper handling of exceptional conditions Newtonsoft#2465

Closed
WonDKim wants to merge 2 commits intoelastic:mainfrom
WonDKim:patch-1
Closed

Fix bug Improper handling of exceptional conditions Newtonsoft#2465
WonDKim wants to merge 2 commits intoelastic:mainfrom
WonDKim:patch-1

Conversation

@WonDKim
Copy link
Copy Markdown

@WonDKim WonDKim commented Oct 11, 2024

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

@cla-checker-service
Copy link
Copy Markdown

cla-checker-service bot commented Oct 11, 2024

💚 CLA has been signed

@github-actions
Copy link
Copy Markdown

👋 @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
Copy Markdown
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