Cap nesting depth in Gem::SafeMarshal::Reader

[davutselcuk]

Opened at your request, from the closing comment on HackerOne #4013093:

Adding a nesting cap to Gem::SafeMarshal::Reader symmetric with the 1000-level cap already in yaml_serializer.rb is worthwhile hardening, so please open it as a pull request.

What this does

Reader#read_element recurses once per nested element and counts nothing, so a stream of nested arrays runs the stack down before any other limit applies. Marshal spends two bytes per level.

This adds the symmetric cap:

MAX_NESTING_DEPTH = 1_000

Past it the reader raises Reader::TooDeeplyNestedError, which is a Reader::Error and therefore a StandardError. That matters for callers already written to tolerate a bad spec, such as Gem::Source#fetch_spec:

spec = begin
         Gem::SafeMarshal.safe_load(spec)
       rescue StandardError
         nil
       end

A SystemStackError descends from Exception, so that rescue never sees it and the intended "skip the bad spec" path does not run. With the cap, it does.

Depth, not width

The counter lives in read_element and unwinds in an ensure, so sibling count is unaffected:

• 998-level nest — parses
• array of 5,000 siblings — parses
• 2,400-level nest — TooDeeplyNestedError

case is untouched; the diff is 12 lines of library code.

Tests

Three added, covering the cap, a nest just under it, and the wide-but-shallow case.

test/rubygems/test_gem_safe_marshal.rb   193 tests, 2218 assertions, 0 failures, 0 errors
test/rubygems/test_gem_source.rb          40 tests,  160 assertions, 0 failures, 0 errors

Also checked a real rack-3.1.8 gemspec fetched from rubygems.org still parses.

🤖 Generated with Claude Code