(Hopefully) Fix use after free

ext/syck/rubyext.c

@@ -645,6 +645,7 @@ SYMID
 rb_syck_load_handler(SyckParser *p, SyckNode *n)
 {
     VALUE obj = Qnil;
+    VALUE node_wrapper;
     struct parser_xtra *bonus = (struct parser_xtra *)p->bonus;
     VALUE resolver = bonus->resolver;
     if ( NIL_P( resolver ) )
@@ -655,7 +656,17 @@ rb_syck_load_handler(SyckParser *p, SyckNode *n)
     /*
      * Create node,
      */
-    obj = rb_funcall( resolver, s_node_import, 1, Data_Wrap_Struct( cNode, syck_node_mark, NULL, n ) );
+    node_wrapper = Data_Wrap_Struct( cNode, syck_node_mark, NULL, n );
+    obj = rb_funcall( resolver, s_node_import, 1, node_wrapper );
+
+    /*
+     * syck_hdlr_add_node frees `n` as soon as we return (handler.c:25, when
+     * the node has no anchor). Ruby may still keep `node_wrapper` reachable
+     * via a conservative GC root on the machine stack, so the next GC would
+     * call syck_node_mark() with a dangling SyckNode*. Zero the wrapper's
+     * data pointer to make the mark a no-op. See ruby/syck#50.
+     */
+    DATA_PTR( node_wrapper ) = NULL;
 
     /*
      * ID already set, let's alter the symbol table to accept the new object
@@ -1413,6 +1424,7 @@ static void
 syck_node_mark(SyckNode *n)
 {
     int i;
+    if ( n == NULL ) return;
     rb_gc_mark_maybe( n->id );
     switch ( n->kind )
     {

test/test_gc.rb

@@ -0,0 +1,33 @@
+require_relative "helper"
+
+# Regression test for ruby/syck#50 - heap-use-after-free in syck_node_mark
+# when GC runs between syck_hdlr_add_node freeing a SyckNode and the Ruby
+# Data wrapper that pointed at it being collected.
+#
+# GC.stress forces a full GC on every allocation, so if the fix regresses
+# this test will segfault (or fire under ASAN/valgrind).
+class TestGC < Test::Unit::TestCase
+  def test_load_under_gc_stress
+    yaml = ({}.tap { |h| 50.times { |i| h["k#{i}"] = "v" * 64 } }).to_yaml
+
+    GC.stress = true
+    begin
+      3.times { Syck.load(yaml) }
+    ensure
+      GC.stress = false
+    end
+  end
+
+  def test_load_many_documents_under_gc_stress
+    docs = Array.new(5) do |i|
+      ({}.tap { |h| 30.times { |k| h["k_#{i}_#{k}"] = "v_#{i}_#{k}" } }).to_yaml
+    end
+
+    GC.stress = true
+    begin
+      docs.each { |d| Syck.load(d) }
+    ensure
+      GC.stress = false
+    end
+  end
+end