Bug #21996: Crash when modifying instance variables during inspect or Marshal dump

• Status: Closed
• Assignee: -
• Target version: -
• Backport: 3.2: UNKNOWN, 3.3: UNKNOWN, 3.4: UNKNOWN, 4.0: UNKNOWN

[jhawthorn (John Hawthorn)]

In #15968 describes an issue where instance variables being modified lead to incorrect Marshal output being generated, which was partially solved by checking for the number of IVs changing and raising an exception. However, if enough IVs removed this could cause the buffer to be moved/re-embedded, but we'd still attempt to read the now removed variables. I've found this reproduces back to Ruby 3.3, but suspect there are other ways to hit issues on older versions.

We've seen the test from #15968 start failing on CI recently (likely because the changes to size pools makes this crash reproducible) https://github.com/ruby/ruby/actions/runs/24247666394/job/70803324392

class Evil
  def initialize(parent)
    @parent = parent
  end

  def marshal_dump
    @parent.instance_variables.each { |v| @parent.remove_instance_variable(v) }
    {}
  end

  def marshal_load(data) = nil
end

obj = Object.new
obj.instance_variable_set(:@evil, Evil.new(obj))
10.times { |i| obj.instance_variable_set(:"@v#{i}", 0) }
Marshal.dump(obj) # SEGV

class Evil
  def initialize(parent)
    @parent = parent
  end

  def inspect
    @parent.instance_variables.each { |v| @parent.remove_instance_variable(v) }
    ""
  end
end

obj = Object.new
obj.instance_variable_set(:@evil, Evil.new(obj))
10.times { |i| obj.instance_variable_set(:"@v#{i}", 0) }
obj.inspect # SEGV

[jhawthorn (John Hawthorn)]

• Related to Bug #15968: Custom marshal_load methods allow object instance variables to "leak" into other objects added

[jhawthorn (John Hawthorn)]

• Status changed from Open to Closed

Applied in changeset git|f2d0ef269b47af7d0e658016903bd8917c32899a.

---

Add and use rb_ivar_foreach_buffered

Previously, rb_ivar_foreach would walk up the shape tree, but yield
instance variables to the callback as it went. If the object shape was
modified during this callback, particularly with removing an instance
variable, it could result in reading free'd memory.

This commit solves this by adding a version which buffering all instance
variable names and values before calling the callback, giving a snapshot
of the object at the time rb_ivar_foreach_buffered is called.

The buffer is made with ALLOCV_N, so the performance difference should
be minimal, and I don't think this method is particuarly heavily used.

[Bug #21996]