mirror of
https://github.com/ruby/ruby.git
synced 2026-10-28 16:36:20 +08:00
Don't retag the shared fstring via TYPE_UCLASS in Marshal.load
This is not a real-world problem. Marshal.dump never emits this payload.
It can only be hand-crafted. Loading untrusted Marshal data is already
unsafe. This fix only avoids false positives from security scanners.
TYPE_UCLASS ('C') relabels the wrapped object's class with RBASIC_SET_CLASS.
It assumes the object was freshly allocated during load. But with
freeze: true, r_leave() interns a wrapped bare String first, and
interning a bare String returns the shared global fstring. So the relabel
hits that global object instead, corrupting every other reference to the
same interned string.
Load the wrapped object as a partial so r_leave() is deferred, and
finalize it only after the class has been set. The String is then
interned as a non-bare String, which keeps the result a distinct object
instead of the shared fstring.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
1 parent
cda3527671
commit
32f06ce98c
2 files changed
+26
-1
Notes:
git
2026-10-27 11:50:44 +00:00
No files matched your search
@@ -2012,7 +2012,11 @@ r_object_for(struct load_arg *arg, bool partial, int *ivp, VALUE klass, VALUE ex
|
||||
hash_new_capa = rb_ident_hash_new_capa;
|
||||
goto type_hash;
|
||||
}
|
||||
v = r_object_for(arg, partial, 0, c, extmod, type);
|
||||
/* Load as a partial and run r_leave() only after the class is set
|
||||
* below: interning a bare frozen String returns the shared global
|
||||
* fstring, and retagging that shared object would corrupt every
|
||||
* other reference to it. */
|
||||
v = r_object_for(arg, true, 0, c, extmod, type);
|
||||
if (RB_SPECIAL_CONST_P(v) || RB_TYPE_P(v, T_OBJECT) || RB_TYPE_P(v, T_CLASS) || RB_TYPE_P(v, T_MODULE)) {
|
||||
goto format_error;
|
||||
}
|
||||
@@ -2027,6 +2031,7 @@ r_object_for(struct load_arg *arg, bool partial, int *ivp, VALUE klass, VALUE ex
|
||||
rb_class_name(c));
|
||||
}
|
||||
RBASIC_SET_CLASS(v, c);
|
||||
v = r_leave(v, arg, partial);
|
||||
}
|
||||
break;
|
||||
|
||||
|
||||
@@ -1123,5 +1123,25 @@ class TestMarshal < Test::Unit::TestCase
|
||||
assert_same objects[1], objects[1].instance_variable_get(:@self)
|
||||
assert_same objects[0].instance_variable_get(:@self), objects[1].instance_variable_get(:@self)
|
||||
end
|
||||
|
||||
class StringSubclass < String
|
||||
end
|
||||
|
||||
def test_uclass_fstring
|
||||
interned = -("fstring".b)
|
||||
assert_instance_of(String, interned)
|
||||
|
||||
# TYPE_UCLASS ('C') wrapping a bare (binary, no ivar) String.
|
||||
payload = "\x04\x08C".b +
|
||||
Marshal.dump(StringSubclass.name.to_sym)[2..] +
|
||||
Marshal.dump("fstring".b)[2..]
|
||||
loaded = Marshal.load(payload, freeze: true)
|
||||
|
||||
assert_instance_of(StringSubclass, loaded)
|
||||
assert_predicate(loaded, :frozen?)
|
||||
assert_not_same(interned, loaded)
|
||||
assert_instance_of(String, interned)
|
||||
assert_instance_of(String, -("fstring".b))
|
||||
end
|
||||
end
|
||||
end
|
||||
Reference in new issue
Block a user