Skip to content

Psych::Emitter#start_document: tag-directive handles and prefixes are aliased as raw char *, and the Strings die inside the loop #812

Description

@jeremy

Summary

start_document_try stores a raw char * into each tag directive's handle and prefix, in
an xcalloc'd array that holds no VALUE. libyaml does not copy those bytes until
yaml_document_start_event_initialize, after the whole loop has run — but the only thing
rooting the Strings the pointers came from is a C local that the next iteration overwrites.
The next iteration can also allocate, and can run arbitrary Ruby. So by the time libyaml
copies, an earlier directive's bytes may have been freed and reused, or relocated by
compaction.

Both failure modes are reachable and I have reproduced both. This is a second, independent
defect in the same file as #811 and is not fixed by fixing that one.

Reproduction

Two, because there are two mechanisms.

1. Relocation. Handles that are already UTF-8 stay put in the tags array;
rb_str_export_to_enc returns the same object. argv pins the array, not its elements, so
an embedded handle can move. Here the second directive's handle is an object whose to_str
runs the compaction, which puts it exactly between the first directive's store and libyaml's
copy:

require "psych"
require "stringio"

class Window
  def to_str
    GC.verify_compaction_references(expand_heap: true, toward: :empty)
    "!w!"
  end
end

# Built in a frame that pops: a String held in a live local is conservatively
# pinned by the machine-stack scan, which masks the bug.
def build_tags
  [["!" + "a" * 98 + "!", "tag:example.com,2026:" + "p" * 79],
   [Window.new, "tag:example.com,2026:w"]]
end

io = StringIO.new
e = Psych::Emitter.new(io)
e.start_stream(Psych::Nodes::Stream::UTF8)
e.start_document([], build_tags, false)
e.scalar("v", nil, nil, true, false, Psych::Nodes::Scalar::ANY)
e.end_document(false)
e.end_stream
puts io.string

Actual:

tag handle must not be empty (RuntimeError)

The vacated slot is zero-filled, so the handle reads as the empty string and libyaml's own
yaml_emitter_analyze_tag_directive rejects it. Expected, and what you get with the
GC.verify_compaction_references line removed — I ran that as a control:

%TAG !aaaa…aaaa! tag:example.com,2026:pppp…pppp
%TAG !w! tag:example.com,2026:w
--- v

2. Use-after-free, no compaction, no coercing object, public API only. When the handle is
not already UTF-8, rb_str_export_to_enc returns a dup, and that dup is rooted by nothing
but the C local. This one needs no cooperating object at all — a plain parse-and-re-emit does
it, because the parser hands back non-UTF-8 Strings when default_internal is set:

require "psych"

Encoding.default_internal = Encoding::ISO_8859_1

handles  = (0...8).map { |i| "!" + (("a".ord + i).chr * 98) + "!" }
prefixes = (0...8).map { |i| "tag:example.com,2026:" + (("a".ord + i).chr * 79) }
src = handles.each_with_index.map { |h, i| "%TAG #{h} #{prefixes[i]}" }.join("\n") +
      "\n--- #{handles[0]}thing\nkey: value\n"

expected = Psych.parse_stream(src).to_yaml     # before the amplifier: correct

GC.stress = true            # amplifier only -- note the compaction count below is 0
bad = 0
errors = Hash.new(0)
20.times do
  out = begin
    Psych.parse_stream(src).to_yaml
  rescue RuntimeError => e
    errors[e.message] += 1
    :raised
  end
  bad += 1 if out != expected
end
GC.stress = false

puts "corrupt: #{bad}/20"
puts "compactions during the run: #{GC.stat(:compact_count)}"
errors.each { |m, n| puts "  raised #{n}x: #{m}" }
corrupt: 20/20
compactions during the run: 0
  raised 20x: duplicate %TAG directive

Zero compactions — this is ordinary GC. "Duplicate" because directive i's freed dup gets
reclaimed by directive i+1's dup, so two entries end up pointing at the same bytes. With the
GC.stress = true line removed, as a control: corrupt: 0/20.

Where the corruption doesn't happen to hit one of libyaml's validity checks it emits silently
wrong output instead. From a longer run of the same program, the first directive's prefix
coming back as the second directive's handle:

%TAG !aaaa…aaaa! %21bbbb…bbbb%21
%TAG !bbbb…bbbb! %21cccc…cccc…

Cause

ext/psych/psych_emitter.c, in start_document_try — lines 196-206 on master, and
194-204 in the released 5.2.6/5.3.1/5.4.0 gems (the two-line offset is the
#ifndef PSYCH_USE_LIBFYAML that master added at the top of the file; the loop itself is
byte-identical):

name  = RARRAY_AREF(tuple, 0);
value = RARRAY_AREF(tuple, 1);
StringValue(name);                                   /* may run arbitrary Ruby: to_str */
StringValue(value);
name  = rb_str_export_to_enc(name, encoding);        /* NEW String unless already UTF-8 */
value = rb_str_export_to_enc(value, encoding);

tail->handle = (yaml_char_t *)StringValueCStr(name); /* char* into an xcalloc'd array   */
tail->prefix = (yaml_char_t *)StringValueCStr(value);/* that holds no VALUE             */

tail++;                                              /* name/value overwritten next lap */

data->head is xcalloc'd and stores no VALUE, so the GC can see nothing there. On the next
iteration name and value are reassigned, and after that:

  • if rb_str_export_to_enc duped — which it does whenever the source encoding isn't UTF-8,
    including for a plain US-ASCII handle, since rb_str_conv_enc_opts dups when
    STR_ENC_GET(str) != to even for an ascii-only string — the dup is unreachable and can be
    collected and its memory reused. No compaction needed.
  • if it returned the same object — the UTF-8 case — the String is still alive via the tags
    array, but array elements are marked movable, so compaction relocates it and tail->handle
    keeps the old address.

Either way the pointer goes stale before yaml_document_start_event_initialize reads it.

The next iteration is also where the trouble comes from: StringValue can call to_str and run
arbitrary Ruby, and rb_str_export_to_enc allocates. So the window is not hypothetical — the
loop creates it itself.

Versions affected

start_document_try is byte-identical in every version I checked, including current master
(8aaf6c2); I extracted the function from each tree and hashed it:

5ecc4fe01c25  psych 5.2.6
5ecc4fe01c25  psych 5.3.1
5ecc4fe01c25  psych 5.4.0
5ecc4fe01c25  ruby/psych master @ 8aaf6c21efb9ddeab0c3fb8cd8064525543e87ef

Reproduction 1 exactly as written above, 3 runs per cell, each against a psych built from the
official released gem, with the loaded .bundle checksummed on every run (libyaml 0.2.5,
arm64-darwin):

ruby psych 5.2.6 psych 5.3.1 psych 5.4.0 control (compaction line removed)
3.4.7 (arm64-darwin25) 3/3 3/3 3/3 3/3 correct
3.4.10 (arm64-darwin23) 3/3 3/3 3/3 3/3 correct
4.0.6 (arm64-darwin23) 3/3 3/3 3/3 3/3 correct

Reproduction 2 exactly as written above, over the same nine cells: corrupt: 20/20,
compactions during the run: 0, raised 20x: duplicate %TAG directive in all nine
. Control
(the GC.stress = true line removed): corrupt: 0/20 in all nine.

Scope, and how often this actually fires

Psych.dump and Object#to_yaml are not affected: lib/psych/visitors/yaml_tree.rb:113
passes a literal [] unconditionally, so the loop is never entered. The exposed path is
lib/psych/visitors/emitter.rb:26, @handler.start_document o.version, o.tag_directives, o.implicit — i.e. emitting a Psych::Nodes::Document that carries %TAG directives, which in
practice means round-tripping a parsed stream (Psych.parse_stream(src).to_yaml) — or driving
Psych::Emitter yourself.

I want to be straight about the rate, because the numbers above are all amplified:

  • With GC.stress, or with anything that runs Ruby inside the loop: 100%, every version,
    every interpreter I tried.
  • Without any amplifier, I could not make it fire. 380,000 round-trips of reproduction 2's
    document, no corruption. But that number is worth less than it looks: at a plausible handle
    size the export dups are small enough to live in the object slot, so they generate no
    malloc_increase and no GC ever landed inside the emit call at all — 300,000 of those
    iterations measure nothing. Pushing the handles past the embedded-string boundary so the dups
    become real allocations, the remaining 80,000 round-trips did put 5,485 ordinary GCs and 3
    compactions inside the window
    , still with no corruption.

So: not observed under ordinary GC, at an upper bound of about one per 5,485 in-call GCs on one
platform. I'm not claiming a rate below that, and I'd rather report the measurement than round it
to "rare" — a use-after-free whose window is one narrow allocation is exactly the kind that shows
up later on a different allocator, a different platform, or a longer document.

Possible fix

Copy the bytes rather than aliasing them. start_document_ensure already exists and already
frees data->head, so there is a cleanup path to hang this on, and rb_ensure already covers a
raise from the middle of the loop:

 struct start_document_data {
     ...
     yaml_tag_directive_t * head;
+    long head_len;
 };
@@
-            tail->handle = (yaml_char_t *)StringValueCStr(name);
-            tail->prefix = (yaml_char_t *)StringValueCStr(value);
+            tail->handle = (yaml_char_t *)ruby_strdup(StringValueCStr(name));
+            tail->prefix = (yaml_char_t *)ruby_strdup(StringValueCStr(value));

             tail++;
+            data->head_len++;
@@ static VALUE start_document_ensure(VALUE d)
     struct start_document_data * data = (struct start_document_data *)d;

+    if (data->head) {
+        yaml_tag_directive_t *t;
+        for (t = data->head; t < data->head + data->head_len; t++) {
+            xfree(t->handle);
+            xfree(t->prefix);
+        }
+    }
     xfree(data->head);

(ruby_strdup is from <ruby/util.h>, which also #defines it as strdup; it allocates
through ruby_xmalloc, so xfree matches. head_len is there because tail is local to
start_document_try and the ensure handler can't see how far the loop got.)

With that applied to the same 5.4.0 source tree, built in the same step so the two can't drift:
both reproductions go green, 3/3 and 20/20, while the instrumentation still shows the subject
relocating and the GCs still landing inside the call — so it's the defect that went away, not
the window. Reproduction 1 pristine 002549db9c… red 3/3, patched 1d26702906… green 3/3.

Keeping the exported Strings alive in a VALUE array instead would fix the use-after-free but
not the relocation, since the recorded pointers would still be stale after a move; copying is
the only one of the obvious options that closes both. Happy to send this as a PR with a
regression test, if you'd like it in this shape.

Context

Found in the same sweep as #811, which is in this same file but is a different defect — that one
is a VALUE handed to libyaml's output handler, this one is char * into String bytes inside
start_document. Fixing either leaves the other. The libfyaml-backend caveat I noted on #811
applies to this fix too.

I checked our own five production applications and the union of their bundles — 989 resolved gem
paths, 446 distinct locked gems — for anything that passes a non-empty tags array to
start_document, or that re-emits a parsed node tree: no call sites. The only gem in that
corpus that emits a Psych::Nodes tree builds its Psych::Nodes::Document with the default
empty tag_directives. So I'm filing this publicly as a correctness bug reachable through the
API you choose to call, not as something an application can be fed. Anyone whose application
does round-trip untrusted YAML through Psych.parse_stream(...).to_yaml should run that grep on
their own corpus rather than inherit my answer, since in that shape the attacker picks the
number and contents of the %TAG directives.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions