Project

General

Profile

Actions

Bug #12988

closed

Calling `inspect` sometimes causes a segv

Added by tenderlovemaking (Aaron Patterson) over 7 years ago. Updated about 7 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 2.4.0dev (2016-10-05 tclass-heaps 56351) [x86_64-darwin16]
[ruby-core:78403]

Description

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.


Files

0001-Stop-reading-past-the-end-of-ivptr-array.patch (1.33 KB) 0001-Stop-reading-past-the-end-of-ivptr-array.patch tenderlovemaking (Aaron Patterson), 11/28/2016 05:58 PM

Updated by normalperson (Eric Wong) over 7 years ago

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.

Updated by nobu (Nobuyoshi Nakada) over 7 years ago

  • 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.

Updated by tenderlovemaking (Aaron Patterson) over 7 years ago

  • Status changed from Assigned to Closed

Should be fixed in r56938

Updated by nobu (Nobuyoshi Nakada) over 7 years ago

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

Updated by usa (Usaku NAKAMURA) over 7 years ago

  • 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.

Updated by nagachika (Tomoyuki Chikanaga) about 7 years ago

  • 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.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0