Bug #12988: Calling `inspect` sometimes causes a segv

• Status: Closed
• Assignee: tenderlovemaking (Aaron Patterson)
• Target version: -
• ruby -v: ruby 2.4.0dev (2016-10-05 tclass-heaps 56351) [x86_64-darwin16]
• Backport: 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

[tenderlovemaking (Aaron Patterson)]

rb_obj_inspect calls rb_ivar_count to find the number of instance variables on an object. rb_ivar_count uses tbl->num_entries on the instance variable index table to determine how far in to the instance variable array it should read. Since the instance variable index table is shared, it may increase in size, but the instance variable array will not.

For example:

class A
  def initialize
    @a = nil
    @b = nil
    @c = nil
    @d = nil
    @e = nil
  end
end

x = A.new
y = x.clone
100.times { |z| x.instance_variable_set(:"@foo#{z}", nil) }
puts y.inspect

x and y share an IV index table. Calling instance_variable_set on x will increase the size of the IV index table. When y.inspect is called, the table size is larger than ROBJECT_IVPTR array for that instance. This means that sometimes calling inspect can segv as it may read memory it shouldn't.

I've attached a patch that fixes this by using the length of the array rather than the size of the IV index table.

[normalperson (Eric Wong)]

[e-mail] wrote:

• Backport: 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED

I think a backport will be required for all supported versions.
I'm surprised this remained undiscovered for so many years.

I've attached a patch that fixes this by using the length of the array rather than the size of the IV index table.

Your fix looks good to me.

[nobu (Nobuyoshi Nakada)]

• Description updated (diff)
• Status changed from Open to Assigned
• Assignee set to tenderlovemaking (Aaron Patterson)
• Backport changed from 2.1: UNKNOWN, 2.2: UNKNOWN, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED

Confirmed the overrun, although it didn't segfault.
Please commit the fix.

[tenderlovemaking (Aaron Patterson)]

• Status changed from Assigned to Closed

Should be fixed in r56938

[nobu (Nobuyoshi Nakada)]

You could close this issue by including [Bug #12988] in the commit message.

[usa (Usaku NAKAMURA)]

• Backport changed from 2.1: REQUIRED, 2.2: REQUIRED, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED

ruby_2_2 r57214 merged revision(s) 56938.

[nagachika (Tomoyuki Chikanaga)]

• Backport changed from 2.1: REQUIRED, 2.2: DONE, 2.3: REQUIRED to 2.1: REQUIRED, 2.2: DONE, 2.3: DONE

ruby_2_3 r57341 merged revision(s) 56938.