You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Packer#write_extension: interior pointer of a coerced temporary held across rb_raise (use-after-move) #401
Packer_write_extension takes a char * into a String that rb_String has just created, and
passes it to rb_raise, which allocates while it formats. If a GC compaction runs during that
allocation the temporary can move, and %s then reads a vacated slot. This is a latent
memory-safety defect on the error path. It is present at HEAD (and in 1.8.4).
Location
ext/msgpack/packer_class.c (line numbers from 1.8.4, identical in 1.8.3):
staticVALUEPacker_write_extension(VALUEself, VALUEobj)
{
...
VALUErb_ext_type=RSTRUCT_GET(obj, 0);
if(!RB_TYPE_P(rb_ext_type, T_FIXNUM)) {
rb_raise(rb_eRangeError, "integer %s too big to convert to `signed char'",
RSTRING_PTR(rb_String(rb_ext_type))); /* :238 */
}
When the type slot is not a Fixnum, rb_String(rb_ext_type) coerces it to a fresh String that
no argument slot holds (argv holds the ExtensionValue struct, not the coerced String). RSTRING_PTR takes an interior pointer into that fresh String, and rb_raise → rb_vsprintf
allocates the message buffer before it reads %s. A compaction in that window relocates the fresh
String; the read lands in the vacated, zero-filled slot.
Reproduction
Reproduces on arm64-darwin, ruby 3.4.7/3.4.10/4.0.6: an ExtensionValue whose type slot is a
non-Fixnum object with an embedded (movable) to_str, packed under GC.stress + GC.auto_compact,
raises RangeError whose %s field is empty — "integer too big to convert to \signed
char'"— 200/200, versus the intact marker on the same run with the type slot already a String (0/200) or with GC off (0/200). Natural rate, noGC.stress`: ~35 corruptions in 300,000 packs.
It is codegen-dependent: on aarch64-linux with Debian gcc 12 the temporary stays in a
conservatively-scanned register across rb_raise and does not move (0/300 while a same-size witness
relocated 300/300), so this build is safe only by accident. A compiler, flag or arch change unpins
it.
Fix
Root the coerced value in a local before raising, so it is a named VALUE on the stack for the
duration of the rb_raise:
VALUEs=rb_String(rb_ext_type);
rb_raise(rb_eRangeError, "integer %"PRIsVALUE" too big to convert to `signed char'", s);
Using %"PRIsVALUE" with the VALUE (rather than %s with RSTRING_PTR) also removes the raw
pointer entirely.
Environment: msgpack 1.8.3 and 1.8.4, ruby 3.4.x, arm64-darwin (fires) and aarch64-linux (latent).
Activity