Skip to content

Conversation

quake
Copy link
Contributor

@quake quake commented Sep 19, 2025

This PR introduces a nesting depth limit to the JSON parser to fix a security vulnerability.

Without a depth limit, parsing a deeply nested JSON string can lead to a stack overflow, which crashes the program. This can be used as a DoS attack.

This is a common security measure found in many other languages and libraries (like Go, Rust, and Python) to ensure safe JSON parsing. This change makes moonbit json parser more robust and secure.

@coveralls
Copy link
Collaborator

coveralls commented Sep 19, 2025

Pull Request Test Coverage Report for Build 1298

Details

  • 14 of 16 (87.5%) changed or added relevant lines in 3 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.01%) to 89.453%

Changes Missing Coverage Covered Lines Changed/Added Lines %
json/parse.mbt 12 13 92.31%
json/types.mbt 0 1 0.0%
Files with Coverage Reduction New Missed Lines %
json/parse.mbt 2 89.36%
Totals Coverage Status
Change from base Build 1297: -0.01%
Covered Lines: 9296
Relevant Lines: 10392

💛 - Coveralls

@peter-jerry-ye peter-jerry-ye force-pushed the quake/json-parse-stack-overflow branch from be6740d to 9b542f8 Compare September 19, 2025 10:37
@quake quake force-pushed the quake/json-parse-stack-overflow branch 2 times, most recently from f6b985d to d70ddbd Compare September 22, 2025 00:52
/// Parse a JSON input string into a Json value, with an optional maximum nesting depth (default is 1024)
pub fn parse(
input : String,
max_nesting_depth? : Int = 1024,
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there any reason for this number, i.e. what's the number picked by the others?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

follow python / java(Jackson), the default limit is around 1000.

@bobzhang bobzhang force-pushed the quake/json-parse-stack-overflow branch from d70ddbd to 7aaf50c Compare September 23, 2025 13:32
@bobzhang bobzhang enabled auto-merge (rebase) September 23, 2025 13:32
@bobzhang bobzhang merged commit d017043 into moonbitlang:main Sep 23, 2025
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants