Allow specifying a maximum recursion for the deserializer#1072
Allow specifying a maximum recursion for the deserializer#1072EdwardCooke merged 8 commits intomasterfrom
Conversation
|
Well that build failure is unfortunate. I suspect a breaking change or something in gitversion. Looks like an environment variable can be set to fix that. Not sure appveyor works, but I'll see if I can set something. |
|
Also, there's a static deserializer builder now that will need to be updated. |
|
PR build should be fixed again. |
|
Thanks for the review, I'll work on the comments in the next days. |
|
I also ran into this security issue recently and was about to file a security report when I saw your PR great timing! Since this is a DoS vulnerability that can crash apps via stack overflow, it’d be grate to prioritize getting this merged and released. After that, maybe a Security Advisory would help the community know and update safely. Really appreciate a quick fix! |
|
I’ve opened a follow-up PR (#1082) that builds on this and adds the missing |
Add max depth handling to StaticDeserializerBuilder (builds on #1072)
|
@aaubry the NuGet API key has expired. Can you refresh it? |
|
#1082 is merged. Can we close this one? |
This adds a
WithMaximumRecursionmethod toDeserializerBuilder. It allows to limit the maximum allowed depth when deserializing a document. This is particularly useful when parsing untrusted YAML as allowing unbounded depth may lead to a stack overflow which might crash the process.The signature of the method is the same as the one on
SerializerBuilder, but in this case there is no default limit as adding one would be a breaking change.I did reuse the existing
RecursionLevelclass to control the recursion but had to make a few adjustments as I felt it was useful to have the start and end markers in the exception. It was also necessary to add an overload to the SerializerState class to enable constructors with parameters.I have added a few tests for both methods since the one on
SerializerBuilderdidn't have any.