Project

General

Profile

Actions

Bug #11674

closed

`local_variables` returns buffer-overflow garbage with methods with > 10 keyword arguments

Added by kjtsanaktsidis (KJ Tsanaktsidis) over 8 years ago. Updated over 8 years ago.

Status:
Closed
Target version:
-
ruby -v:
ruby 2.2.3p173 (2015-08-18 revision 51636) [x86_64-darwin15]
[ruby-core:71437]

Description

The following program appears to demonstrate a buffer overflow in rb_f_local_variables

def with_kwargs_10(v1:, v2:, v3:, v4:, v5:, v6:, v7:, v8:, v9:, v10:)
    p local_variables
end

def with_kwargs_11(v1:, v2:, v3:, v4:, v5:, v6:, v7:, v8:, v9:, v10:, v11:)
    p local_variables
end

def with_kwargs_12(v1:, v2:, v3:, v4:, v5:, v6:, v7:, v8:, v9:, v10:, v11:, v12:)
    p local_variables
end

def with_args_11(v1,v2,v3,v4,v5,v6,v7,v8,v9,v10,v11)
    p local_variables
end

with_kwargs_10(v1:1,v2:2,v3:3,v4:4,v5:5,v6:6,v7:7,v8:8,v9:9,v10:10)
with_kwargs_11(v1:1,v2:2,v3:3,v4:4,v5:5,v6:6,v7:7,v8:8,v9:9,v10:10,v11:11)
with_kwargs_12(v1:1,v2:2,v3:3,v4:4,v5:5,v6:6,v7:7,v8:8,v9:9,v10:10,v11:11,v12:12)
with_args_11(1,2,3,4,5,6,7,8,9,10,11)

Output:

[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11, :!]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11, :v12, :"\""]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11]

Expected output:

[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11, :v12]
[:v1, :v2, :v3, :v4, :v5, :v6, :v7, :v8, :v9, :v10, :v11]

There appears to be a buffer overflow, because the symbol :""" is next in ASCII order to :!

I'm not familiar with the MRI interpreter internals; I spent a few hours trying to debug the problem but to no avail. It appears that in vm_eval.c:2072, cfp->iseq->local_table_size is 12 (in with_kwargs_11) even though there are only 11 kwargs and no other locals. However, that's as far as I got.

Updated by 0x0dea (D.E. Akers) over 8 years ago

Well, this is very strange indeed. The problem does start at 11 keyword arguments and continues to present all the way up to 25, but then 26 to 35 keyword arguments are all kosher, then 36 to 42 have the problem, and then it doesn't happen again until 69. I have no idea what's going on, but here's the program I used to determine these ranges:

p 100.times.select { |n|
  vars = n.times.map { |i| :"v#{i}" }
  eval "def foo #{vars.map { |v| "#{v}: 1" } * ?,}
          local_variables
        end"
  foo != vars
}

# => [11, 12, 13, 14, 15, 16, 17, 18, 19, 20, 21, 22, 23, 24, 25, 36, 37, 38, 39, 40, 41, 42, 69, 70, 71, 72, 74]

Updated by ko1 (Koichi Sasada) over 8 years ago

  • Assignee set to ko1 (Koichi Sasada)

Updated by Hanmac (Hans Mackowiak) over 8 years ago

i wanted to see what symbols does appear:

 p 200.times.map { |n|
                     vars = n.times.map { |i| :"v#{i}" }
           eval "def foo #{vars.map { |v| "#{v}: 1" } * ?,}
         local_variables
       end"
           foo - vars
       }
#=> [[], [], [], [], [], [], [], [], [], [], [], [:!], [:"\""], [:"#"], [:"$"], [:%], [:&], [:"'"], [:"("], [:")"], [:*], [:+], [:","], [:-], [:"."], [:/], [], [], [], [], [], [], [], [], [], [], [:":"], [:";"], [:<], [:"="], [:>], [:"?"], [:"@"], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [:"["], [:"\\"], [:"]"], [:^], [], [:`], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [], [:"{"], [:|], [:"}"], [:~], [], [:".."], [:"..."], [:+@], [:-@], [:**], [], [:<=>], [:<<], [:>>], [:<=], [:>=], [:==], [:===], [:!=], [:=~], [:!~], [:[]], [:[]=], [:"::"], [], [:"&&"], [:"||"], [:"&."], [:""], [:empty?], [:eql?], [:respond_to?], [:respond_to_missing?], [:"<IFUNC>"], [:"<CFUNC>"], [:"core#set_method_alias"], [:"core#set_variable_alias"], [:"core#undef_method"], [:"core#define_method"], [:"core#define_singleton_method"], [:"core#set_postexe"], [:"core#hash_from_ary"], [:"core#hash_merge_ary"], [:"core#hash_merge_ptr"], [:"core#hash_merge_kwd"], [], [], [], [:freeze], [:inspect], [:intern], [:object_id], [:const_missing], [:method_missing], [:method_added], [:singleton_method_added], [:method_removed], [:singleton_method_removed], [:method_undefined], [:singleton_method_undefined], [:length], [:size], [:gets], [:succ], [:each], [:proc], [:lambda], [:send], [:__send__], [:__attached__], [:initialize], [:initialize_copy], [:initialize_clone], [:initialize_dup], [:to_int], [:to_ary], [:to_str], [:to_sym], [:to_hash], [:to_proc], [:to_io], [:to_a], [:to_s], [:to_i], [:bt], [:bt_locations], [:call], [:mesg], [:exception], [:_], [:__autoload__], [:__classpath__], [:__tmp_classpath__], [:__classid__], [:to_f], [:dig], [:BasicObject], [:Object], [:Module]]

and because they look to well-formed to be random buffer-overflow,
it seems that they are coming from "defs/id.def"
but i don't know why

Actions #4

Updated by nobu (Nobuyoshi Nakada) over 8 years ago

  • Status changed from Open to Closed

Applied in changeset r52536.


vm_eval.c: skip internal names

  • vm_eval.c (local_var_list_add): skip internal local variable
    name by its type but not if it has a name. internal local
    variable names are just unique per frame, not globally.
    [ruby-core:71437] [Bug #11674]

Updated by nagachika (Tomoyuki Chikanaga) over 8 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: UNKNOWN to 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: REQUIRED

Updated by kjtsanaktsidis (KJ Tsanaktsidis) over 8 years ago

Thanks, that was quick! I don't really understand what the above patch is doing, but it looks like you're just filtering out internal local vars as part of the local_variable method implementation. Why is exactly one of these internal symbols sitting in the local variable table, only under some circumstances related to keyword arguments? I guess I don't understand how this problem can happen in the first place - can you explain it to me?

Updated by nagachika (Tomoyuki Chikanaga) over 8 years ago

  • Backport changed from 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: REQUIRED to 2.0.0: UNKNOWN, 2.1: UNKNOWN, 2.2: DONE

Backported into ruby_2_2 branch at r52787.

Actions

Also available in: Atom PDF

Like0
Like0Like0Like0Like0Like0Like0Like0