Ruby: fix use-after-free of map keys aliasing a temporary String (#29026)

Fixes #29023.

`Map#[]=` and `Message.new(map_field: {...})` build the map key as a `upb_StringView`
aliasing a Ruby String, then convert the value before `upb_Map_Set` copies the key. The
value conversion allocates, so it can trigger GC inside that window.

The aliased String is frequently a **temporary**: `Convert_RubyToUpb` replaces the caller's
object when the key is a Symbol (via `to_s`) or a String not already tagged UTF-8 (via
`Convert_CheckStringUtf8`), and nothing references the result once it returns. When GC
collects it, the freed block goes straight back to the next `upb_Arena_Malloc`, which
memcpys the *value* into it — leaving a silently corrupted key holding unrelated heap bytes,
tagged UTF-8 while containing invalid UTF-8, which then propagates into `encode`/`to_json`.

## The fix

Pass the arena at both insertion sites, so the key is copied before anything can allocate.

The lookup paths (`Map_index`, `Map_has_key`, `Map_delete`) keep the `NULL` fast path — they
consume the key immediately with no allocation in between, which is exactly the precondition
`Convert_StringData`'s comment describes. I reworded that comment, since it read as though
the aliasing were unconditionally safe; it holds for three of its five callers and not for
the two that insert.

Cost is one arena allocation per insert for string-typed keys. Non-string keys don't reach
`Convert_StringData` at all.

## Trigger

Needs both:

- a key that is a **Symbol**, or a String not already tagged UTF-8 — `ASCII-8BIT` is the
  common case for anything read from a socket, a file, `Marshal`, or `String#pack`; and
- a value whose conversion allocates (a Symbol, or a non-UTF-8 String).

Plain UTF-8 keys are unaffected, which is presumably why this has gone unnoticed.

## Verification

Reproduces under **ordinary GC**, no `GC.stress` required — one corrupted key across 150k
iterations (0/50k, 0/50k, 1/50k), versus 100/100 with stress. That second number is an
existence proof rather than a rate.

Added regression tests to `ruby/tests/gc_test.rb` covering string keys, Symbol keys, and the
map-field kwarg path. Verified red/green against the same tree:

| ext build | new tests |
|---|---|
| unpatched `main` | 3 tests, **3 failures** |
| with this change | 3 tests, 300 assertions, **0 failures** |

Full Ruby suite green with the change on ruby 4.0.6 / arm64-darwin — `basic.rb` (133 tests,
157,864 assertions), `basic_proto2.rb` (93), `repeated_field_test.rb` (40),
`encode_decode_test.rb`, `memory_test.rb`, `object_cache_test.rb`, `well_known_types_test.rb`,
`service_test.rb`, `oom_test.rb`, `multi_level_nesting_test.rb` — 0 failures, 0 errors.

Reported separately via the channel in `SECURITY.md`, since this is a memory-safety issue in
an OT0 repository.

Closes #29026

COPYBARA_INTEGRATE_REVIEW=https://github.com/protocolbuffers/protobuf/pull/29026 from jeremy:ruby-map-key-use-after-free e11cc7dfe2
PiperOrigin-RevId: 961236703

ruby/ext/google/protobuf_c/convert.c

@@ -28,7 +28,12 @@ static upb_StringView Convert_StringData(VALUE str, upb_Arena* arena) {
     memcpy(ptr, RSTRING_PTR(str), RSTRING_LEN(str));
     ret.data = ptr;
   } else {
-    // Data is only needed temporarily (within map lookup).
+    // Alias the Ruby String's bytes instead of copying them. Only valid when
+    // the result is consumed before anything can allocate: Convert_RubyToUpb
+    // may have converted `str` to a temporary that nothing else references, so
+    // any GC between here and the read can free or move it. The map lookup
+    // paths (Map_index, Map_has_key, Map_delete) satisfy this; insertion paths
+    // must pass an arena.
     ret.data = RSTRING_PTR(str);
   }
   ret.size = RSTRING_LEN(str);

ruby/ext/google/protobuf_c/map.c

@@ -434,8 +434,13 @@ static VALUE Map_index(VALUE _self, VALUE key) {
 static VALUE Map_index_set(VALUE _self, VALUE key, VALUE val) {
   Map* self = ruby_to_Map(_self);
   upb_Arena* arena = Arena_get(self->arena);
+  // The key must be copied into the arena, not aliased. Converting the value
+  // below can allocate a Ruby object and therefore trigger GC, which may free
+  // or move the String the key would otherwise point into. Passing NULL here is
+  // only safe when the key is consumed before any allocation, as in the lookup
+  // paths (Map_index, Map_has_key, Map_delete).
   upb_MessageValue key_upb =
-      Convert_RubyToUpb(key, "", Map_keyinfo(self), NULL);
+      Convert_RubyToUpb(key, "", Map_keyinfo(self), arena);
   upb_MessageValue val_upb =
       Convert_RubyToUpb(val, "", self->value_type_info, arena);
 

ruby/ext/google/protobuf_c/message.c

@@ -527,7 +527,10 @@ typedef struct {
 static int Map_initialize_kwarg(VALUE key, VALUE val, VALUE _self) {
   MapInit* map_init = (MapInit*)_self;
   upb_MessageValue k, v;
-  k = Convert_RubyToUpb(key, "", map_init->key_type, NULL);
+  // Copy the key into the arena rather than aliasing it: building the value
+  // below allocates, which can trigger GC and free or move the String the key
+  // would otherwise point into.
+  k = Convert_RubyToUpb(key, "", map_init->key_type, map_init->arena);
 
   if (map_init->val_type.type == kUpb_CType_Message && TYPE(val) == T_HASH) {
     const upb_MiniTable* t =

ruby/tests/gc_test.rb

@@ -104,4 +104,57 @@ class GCTest < Test::Unit::TestCase
     GC.stress = old_gc
     puts "passed"
   end
+
+  # Regression test: the map key must be copied into the arena, not aliased.
+  #
+  # Convert_RubyToUpb returns a *temporary* String for a key that is a Symbol or
+  # is not already tagged UTF-8. Converting the value afterwards allocates, which
+  # can trigger GC and free that temporary before upb_Map_Set copies the key --
+  # leaving a silently corrupted key holding unrelated heap bytes.
+  def assert_map_keys_survive_gc(&builder)
+    old_gc = GC.stress
+    GC.stress = true
+    begin
+      100.times do
+        # Non-UTF-8 key and value: the key conversion allocates a temporary, and
+        # the value conversion allocates again, opening the window.
+        key = ("K" * 5000).dup.force_encoding("ISO-8859-1") +
+              "\xE9".dup.force_encoding("ISO-8859-1")
+        value = ("V" * 5000).dup.force_encoding("ISO-8859-1") +
+                "\xE9".dup.force_encoding("ISO-8859-1")
+        assert_equal [key.encode("UTF-8")], builder.call(key, value).keys
+      end
+    ensure
+      GC.stress = old_gc
+    end
+  end
+
+  def test_map_string_key_not_corrupted_by_gc
+    assert_map_keys_survive_gc do |key, value|
+      map = Google::Protobuf::Map.new(:string, :string)
+      map[key] = value
+      map
+    end
+  end
+
+  def test_map_symbol_key_not_corrupted_by_gc
+    old_gc = GC.stress
+    GC.stress = true
+    begin
+      100.times do
+        map = Google::Protobuf::Map.new(:string, :string)
+        map[:some_symbol_key] = :some_symbol_value
+        assert_equal ["some_symbol_key"], map.keys
+      end
+    ensure
+      GC.stress = old_gc
+    end
+  end
+
+  def test_map_field_kwarg_key_not_corrupted_by_gc
+    assert_map_keys_survive_gc do |key, value|
+      A::B::C::TestMessage.new(:map_string_string => { key => value })
+        .map_string_string
+    end
+  end
 end