aboutsummaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorDaniel Colson <danieljamescolson@gmail.com>2022-07-02 20:28:39 -0400
committerJohn Hawthorn <john@hawthorn.email>2022-07-20 10:38:44 -0700
commit32e406d6d3c3ded9160298c4475c1aa188360b07 (patch)
tree58d7062459106012d4549296c3949bf639d5235a
parent5871ecf956711fcacad7c03f2aef95115ed25bc4 (diff)
downloadruby-32e406d6d3c3ded9160298c4475c1aa188360b07.tar.gz
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@github.com>
-rw-r--r--gc.c2
-rw-r--r--internal/symbol.h1
-rw-r--r--symbol.c6
-rw-r--r--test/ruby/test_objectspace.rb5
4 files changed, 13 insertions, 1 deletions
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)
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));
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)
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
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)