Add StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000) [CVE-2025-52999]
#943
Conversation
pjfanning
changed the title
|
Yes; this is along lines of what I was originally thinking. I think there are some benefits in avoiding possibility of accidentally forgotten "closing" of nesting levels, to avoid bugs, |
| public void validateNestingDepth(int depth) throws StreamConstraintsException | ||
| { | ||
| if (depth > _maxNestingDepth) { | ||
| throw new StreamConstraintsException(String.format("Depth (%d) exceeds the maximum allowed nesting depth (%d)", |
There was a problem hiding this comment.
Not related to this PR specifically, but one thing we may want to do afterwards is to add something to note that StreamReadConstraints settings control this -- just add some more verbiage in exception message.
But I think that makes sense as a separate PR once we have nesting depth checks covered.
There was a problem hiding this comment.
StreamConstraintsException has Javadoc that describes what the exception is about. I'm not sure there is much to be gained by making the message more verbose.
There was a problem hiding this comment.
My perspective is that when someone sees a stack trace in log files, it is nice to have slightly more information on context to give Ops people bit more clue on what might be happening.
But I guess minimum level certainly is to have information on exception class itself, pointing to constraints settings. I'll see if I can add something to relevant JAva class(es) in jackson-core.
|
@pjfanning Let me know if and when you are content with the base -- I think this is the last "must-have" feature before 2.15 release candidates. |
pjfanning
changed the title
@cowtowncoder I'm happy to get this merged more or less as it is. There is uptake needed in numerous other Jackson modules so it would be good to make a start on that. |
|
@pjfanning Merged into 2.15, up to master -- will make some smaller changes (increase test coverage, add that one constructor). Note: |
cowtowncoder
changed the title
StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000)
cowtowncoder
added
2.15
processing-limits
cowtowncoder
changed the title
StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000)StreamReadConstraints.maxNestingDepth() to constraint max nesting depth (default: 1000) [CVE-2025-52999]
|
It looks like the CVE makes a bad assertion. How can the tokenizer run into a StackOverflowError? |
|
@wolfc It can't, this does require use of databind (or similar) to read. But since the fix is in streaming level (prevent reading of highly nested content), so is CVE. Put another way, to fix the issue that is triggered via So yes, it is possible to use just |
|
So technically it is a feature introduced in |
|
@wolfc Yeah, adding depth checks for every deserializer (...and serializer for another limit) would be lots of overhead on writing & maintaining code, as well as for processing overhead. Nesting checks for |
Basically #926 but depth is tracked on the JsonStreamContext instead. I have built a jackson-databind enhancement based on this.
CVE-2025-52999 has been raised to highlight the security issue that this fixes. Without this limit, you can run into a StackoverflowError if you parse JSON that is very deeply nested. The StackoverflowError issue happens in jackson-databind but this change in jackson-core stops it from happening unless you increase the StreamReadConstraints.maxNestingDepth() to a high number.
This change means that you will get a checked Exception instead (StreamConstraintsException).