[前][次][番号順一覧][スレッド一覧]

ruby-changes:72614

From: Daniel <ko1@a...>
Date: Thu, 21 Jul 2022 02:39:09 +0900 (JST)
Subject: [ruby-changes:72614] 32e406d6d3 (master): Ensure _id2ref finds symbols with the correct type

https://git.ruby-lang.org/ruby.git/commit/?id=32e406d6d3

From 32e406d6d3c3ded9160298c4475c1aa188360b07 Mon Sep 17 00:00:00 2001
From: Daniel Colson <danieljamescolson@g...>
Date: Sat, 2 Jul 2022 20:28:39 -0400
Subject: Ensure _id2ref finds symbols with the correct type

Prior to this commit it was possible to call `ObjectSpace._id2ref` with
an offset static symbol object_id and get back a new, incorrectly tagged
symbol:

```
> sensible_sym = ObjectSpace._id2ref(:a.object_id)
=> :a
> nonsense_sym = ObjectSpace._id2ref(:a.object_id + 40)
=> :a
> sensible_sym == nonsense_sym
=> false
```

`nonsense_sym` ends up tagged with `RUBY_ID_INSTANCE` instead of
`RB_ID_LOCAL`. That means we can do silly things like:

```
> foo = Object.new
> foo.instance_variable_set(:a, 123)
(irb):2:in `instance_variable_set': `a' is not allowed as an instance variable name (NameError)
> foo.instance_variable_set(ObjectSpace._id2ref(:a.object_id + 40), 123)
=> 123
> foo.instance_variables
=> [:a]
```

This was happening because `get_id_entry` ignores the tag bits when
looking up the symbol. So `rb_id2str(symid)` would return a value and
then we'd continue on with the nonsense `symid`.

This commit prevents the situation by checking that the `symid` actually
matches what we get back from `get_id_entry`. Now we get a `RangeError`
for the nonsense id:

```
> ObjectSpace._id2ref(:a.object_id)
=> :a
> ObjectSpace._id2ref(:a.object_id + 40)
(irb):1:in `_id2ref': 0x000000000013f408 is not symbol id value (RangeError)
```

Co-authored-by: John Hawthorn <jhawthorn@g...>
---
 gc.c                          | 2 +-
 internal/symbol.h             | 1 +
 symbol.c                      | 6 ++++++
 test/ruby/test_objectspace.rb | 5 +++++
 4 files changed, 13 insertions(+), 1 deletion(-)

diff --git a/gc.c b/gc.c
index 84d9b706fa..6246c8d637 100644
--- a/gc.c
+++ b/gc.c
@@ -4652,7 +4652,7 @@ id2ref(VALUE objid) https://github.com/ruby/ruby/blob/trunk/gc.c#L4652
         if ((ptr % sizeof(RVALUE)) == (4 << 2)) {
             ID symid = ptr / sizeof(RVALUE);
             p0 = (void *)ptr;
-            if (rb_id2str(symid) == 0)
+            if (!rb_static_id_valid_p(symid))
                 rb_raise(rb_eRangeError, "%p is not symbol id value", p0);
             return ID2SYM(symid);
         }
diff --git a/internal/symbol.h b/internal/symbol.h
index 4f041330f9..30c81ea004 100644
--- a/internal/symbol.h
+++ b/internal/symbol.h
@@ -30,6 +30,7 @@ PUREFUNC(int rb_is_attrset_sym(VALUE sym)); https://github.com/ruby/ruby/blob/trunk/internal/symbol.h#L30
 ID rb_make_internal_id(void);
 ID rb_make_temporary_id(size_t n);
 void rb_gc_free_dsymbol(VALUE);
+int rb_static_id_valid_p(ID id);
 
 #if __has_builtin(__builtin_constant_p)
 #define rb_sym_intern_ascii_cstr(ptr) \
diff --git a/symbol.c b/symbol.c
index dc7d72cb90..94fa09d19c 100644
--- a/symbol.c
+++ b/symbol.c
@@ -481,6 +481,12 @@ get_id_entry(ID id, const enum id_entry_type t) https://github.com/ruby/ruby/blob/trunk/symbol.c#L481
     return get_id_serial_entry(rb_id_to_serial(id), id, t);
 }
 
+int
+rb_static_id_valid_p(ID id)
+{
+    return STATIC_ID2SYM(id) == get_id_entry(id, ID_ENTRY_SYM);
+}
+
 static inline ID
 rb_id_serial_to_id(rb_id_serial_t num)
 {
diff --git a/test/ruby/test_objectspace.rb b/test/ruby/test_objectspace.rb
index e0f9eecd11..3c912fe300 100644
--- a/test/ruby/test_objectspace.rb
+++ b/test/ruby/test_objectspace.rb
@@ -65,6 +65,11 @@ End https://github.com/ruby/ruby/blob/trunk/test/ruby/test_objectspace.rb#L65
     assert_raise_with_message(TypeError, msg) {ObjectSpace._id2ref(Object.new)}
   end
 
+  def test_id2ref_invalid_symbol_id
+    msg = /is not symbol id value/
+    assert_raise_with_message(RangeError, msg) { ObjectSpace._id2ref(:a.object_id + 40) }
+  end
+
   def test_count_objects
     h = {}
     ObjectSpace.count_objects(h)
-- 
cgit v1.2.1


--
ML: ruby-changes@q...
Info: http://www.atdot.net/~ko1/quickml/

[前][次][番号順一覧][スレッド一覧]