feat(javascript): limit the depth of deserialize & serialize#3382
feat(javascript): limit the depth of deserialize & serialize#3382miantalha45 wants to merge 4 commits intoapache:mainfrom
Conversation
| ${this.readTypeInfo()} | ||
| ${this.read(assignStmt, refState)}; | ||
| fory.incReadDepth(); | ||
| try { |
There was a problem hiding this comment.
the try finally will introduce extra cost. Please remove try finally, and refactor serialize/deserializae entry point, create resetWrite/resetRead methods, move code like:
this.referenceResolver.resetRead();
this.binaryReader.reset(bytes);
this.typeMetaResolver.resetRead();
this.metaStringResolver.resetRead();
into resetRead()
and move smililiar methods into resetWrite.
Then set depth to 0 in resetRead() and resetWrite.
Current referenceResolver/typeMetaResolver/metaStringResolver may lack some such methods, please take java as referencev and add to javascript
… overhead - Remove try/finally blocks from generated read code for performance - Create resetRead() and resetWrite() methods in Fory class - Move all reset logic into these methods (depth, resolvers, etc) - Add resetRead() and resetWrite() methods to all resolver classes - Depth is now reset to 0 at the start of each deserialization - Depth accumulates during deserialization without manual decrement - Keeps zero-cost performance while maintaining security protection - Update tests to match new behavior: depth resets at start of each call
| fory.decReadDepth(); | ||
| } | ||
| ${this.readTypeInfo()} | ||
| ${this.read(assignStmt, refState)}; |
There was a problem hiding this comment.
Where you decReadDepth?
There was a problem hiding this comment.
I am also curious why the tests don't fail? The tests for depth is also wrong
|
Please do not write filename such as |
2 participants
What does this PR do?
Adds depth limiting for deserialization to prevent stack overflow and denial-of-service attacks from maliciously crafted deeply nested data structures.
Why is this needed?
Without depth limits, an attacker could send deeply nested serialized data that causes stack overflow during deserialization, crashing the application or causing resource exhaustion.
Implementation
maxDepthconfig option (default: 50, minimum: 2)Usage
Consistency
Follows the same pattern as Java and Python implementations for cross-language alignment.
Fixes #3335