fix(rube): B1/B2 - close both live detector bypasses, auxiliary owns traversal

Duplicate ivar names deleted subtrees and five of six readers threw away the
class-name node, so a gadget in either position was invisible to the detector
while Marshal.load still fired it. Both had working proofs; the suite was green
the whole time.

The parse graph now has exactly one traversal owner. read_class_name and
read_instance_variables push every class-name node, ivar name and ivar value
into auxiliary, and Node#each no longer walks instance_variables_map. The map
stays as a lookup convenience with last-write-wins semantics, it just is not
load-bearing for security any more. Walking both would have double-counted
every ivar value.

Corpus entries take an optional allowlist. Without one every case ran through
an empty strict allowlist where any class name rejects, which is why the corpus
could not express B2 at all. The 40 existing cases default to [] and are
unchanged.

At the tag level there are seven class-name slots, not six: o S u U d C e, and
only o retained its node. u, U and d are themselves sink tags so a corpus case
there can never fail; those three are asserted at the parser level instead and
a test pins the exclusion as deliberate rather than an oversight.

Verified by mutation, since green means nothing on this project. Dropping the
class-name push, dropping the ivar value push, and restoring the map walk each
now fail 2, 4 and 3 tests. The first two previously survived the entire suite.

119 tests from 110, 48 corpus cases from 40. test, control, exploit, detector,
target and matrix all pass.

PROJECTS/beginner/deserialization-gadget-lab/justfile

@@ -19,7 +19,7 @@ test:
     {{run_ro}} ruby -Ilib -Itest test/corpus_test.rb
 
 corpus:
-    {{run_ro}} ruby -Ilib -Itest -e 'require "rube"; require "support/adversarial_corpus"; d = Rube::Marshal::BoundaryDetector.new; Rube::AdversarialCorpus::CASES.each { |k| dec = d.inspect_stream(k[:bytes]); puts format("  %-30s %-6s %s", k[:name], dec.accepted? ? "accept" : "reject", dec.reason.to_s[0, 60]) }'
+    {{run_ro}} ruby -Ilib -Itest -e 'require "rube"; require "support/adversarial_corpus"; Rube::AdversarialCorpus::CASES.each { |k| d = Rube::Marshal::BoundaryDetector.new(allowed_class_names: k[:allowed]); dec = d.inspect_stream(k[:bytes]); puts format("  %-38s %-6s %-10s %s", k[:name], dec.accepted? ? "accept" : "reject", k[:allowed].join(","), dec.reason.to_s[0, 52]) }'
 
 scan namespace="":
     {{run_ro}} ruby -Ilib -e 'require "rube"; ns = "{{namespace}}"; r = Rube::Scanner.new(namespace: ns.empty? ? nil : ns).scan; puts "modules=#{r.scanned_modules} candidates=#{r.candidates.length} gated=#{r.gated.length} reachable=#{r.reachable.length}"; puts; r.reachable.each { |c| puts format("  %-10s %-46s %s", c.gate, c.to_s, c.source_location) }'

PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb

@@ -34,7 +34,6 @@ module Rube
 
         yield self
         children.each { |child| child.each(&block) }
-        instance_variables_map.each_value { |child| child.each(&block) }
         auxiliary.each { |child| child.each(&block) }
       end
     end

PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb

@@ -208,53 +208,58 @@ module Rube
         pair
       end
 
-      def read_ivar(tag, depth)
-        inner = read_value(depth)
-        read_entry_count(ROLE_IVAR).times do
-          name = read_value(depth + 1)
-          inner.auxiliary << name
-          inner.instance_variables_map[name.value] = read_value(depth + 1)
-        end
-        inner
-      end
-
-      def read_object(tag, depth)
-        node = register(Node.new(type: :object, tag: tag))
-        class_node = read_value(depth + 1)
+      def read_class_name(node, depth)
+        class_node = read_value(depth)
         node.class_name = class_node.value.to_s
         node.auxiliary << class_node
+        node
+      end
+
+      def read_instance_variables(node, depth)
         read_entry_count(ROLE_IVAR).times do
           name = read_value(depth + 1)
+          value = read_value(depth + 1)
           node.auxiliary << name
-          node.instance_variables_map[name.value] = read_value(depth + 1)
+          node.auxiliary << value
+          node.instance_variables_map[name.value] = value
         end
         node
       end
 
+      def read_ivar(tag, depth)
+        read_instance_variables(read_value(depth), depth)
+      end
+
+      def read_object(tag, depth)
+        node = register(Node.new(type: :object, tag: tag))
+        read_class_name(node, depth + 1)
+        read_instance_variables(node, depth)
+      end
+
       def read_struct(tag, depth)
         node = register(Node.new(type: :struct, tag: tag))
-        node.class_name = read_value(depth + 1).value.to_s
+        read_class_name(node, depth + 1)
         read_entry_count(ROLE_STRUCT).times { node.children << read_pair(depth) }
         node
       end
 
       def read_userdef(tag)
         node = Node.new(type: :userdef, tag: tag)
-        node.class_name = read_value(1).value.to_s
+        read_class_name(node, 1)
         node.value = read_counted_bytes
         node
       end
 
       def read_usermarshal(tag, depth)
         node = register(Node.new(type: :usermarshal, tag: tag))
-        node.class_name = read_value(depth + 1).value.to_s
+        read_class_name(node, depth + 1)
         node.children << read_value(depth + 1)
         node
       end
 
       def read_wrapped(tag, type, depth)
         node = register(Node.new(type: type, tag: tag))
-        node.class_name = read_value(depth + 1).value.to_s
+        read_class_name(node, depth + 1)
         node.children << read_value(depth + 1)
         node
       end

PROJECTS/beginner/deserialization-gadget-lab/test/corpus_test.rb

@@ -6,8 +6,8 @@ require_relative "support/adversarial_corpus"
 
 module Rube
   class CorpusTest < Minitest::Test
-    def detector
-      Marshal::BoundaryDetector.new
+    def detector(allowed = [])
+      Marshal::BoundaryDetector.new(allowed_class_names: allowed)
     end
 
     def test_corpus_covers_both_verdicts
@@ -23,7 +23,7 @@ module Rube
 
     def test_every_corpus_case_matches_its_verdict
       disagreements = AdversarialCorpus::CASES.filter_map do |kase|
-        decision = detector.inspect_stream(kase[:bytes])
+        decision = detector(kase[:allowed]).inspect_stream(kase[:bytes])
         expected = kase[:verdict] == AdversarialCorpus::VERDICT_ACCEPT
         next if decision.accepted? == expected
 
@@ -35,7 +35,7 @@ module Rube
 
     def test_parser_never_raises_outside_the_stream_error_hierarchy
       leaks = AdversarialCorpus::CASES.filter_map do |kase|
-        detector.inspect_stream(kase[:bytes])
+        detector(kase[:allowed]).inspect_stream(kase[:bytes])
         nil
       rescue StandardError => e
         "#{kase[:name]}: #{e.class}"
@@ -43,5 +43,44 @@ module Rube
 
       assert_empty leaks, "unhandled exceptions escaped the detector:\n  #{leaks.join("\n  ")}"
     end
+
+    def test_allowlisted_cases_would_still_be_rejected_without_their_allowlist
+      allowlisted = AdversarialCorpus::CASES.reject { |kase| kase[:allowed].empty? }
+      refute_empty allowlisted
+
+      leaks = allowlisted.reject { |kase| detector.inspect_stream(kase[:bytes]).rejected? }
+      assert_empty leaks.map { |kase| kase[:name] },
+                   "an allowlist must widen what is accepted, never what is rejected"
+    end
+
+    def test_every_class_name_slot_carries_byte_identical_gadget
+      blind = AdversarialCorpus::CLASS_NAME_SLOTS.reject do |_slot, bytes|
+        bytes.include?(AdversarialCorpus::CLASS_NAME_GADGET)
+      end
+
+      assert_empty blind.keys,
+                   "slot verdicts only compare if the embedded gadget bytes are identical"
+    end
+
+    def test_class_name_slots_whose_host_is_not_itself_a_sink_are_all_in_the_corpus
+      names = AdversarialCorpus::CASES.map { |kase| kase[:name] }
+      missing = AdversarialCorpus::CLASS_NAME_SLOTS_WITH_NON_SINK_HOST.reject do |slot|
+        names.include?(:"class_name_slot_#{slot}")
+      end
+
+      assert_empty missing, "these slots can flip a detector verdict and must be corpus cases"
+    end
+
+    def test_sink_hosted_class_name_slots_are_excluded_from_the_corpus
+      hosts = AdversarialCorpus::CLASS_NAME_SLOTS.keys -
+              AdversarialCorpus::CLASS_NAME_SLOTS_WITH_NON_SINK_HOST
+      names = AdversarialCorpus::CASES.map { |kase| kase[:name] }
+
+      hosts.each do |slot|
+        refute_includes names, :"class_name_slot_#{slot}",
+                        "#{slot} rejects on its own tag either way, so a corpus case cannot fail"
+        assert detector.inspect_stream(AdversarialCorpus::CLASS_NAME_SLOTS[slot]).rejected?
+      end
+    end
   end
 end

PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb

@@ -2,6 +2,7 @@
 # parser_test.rb
 
 require_relative "../test_helper"
+require_relative "../support/adversarial_corpus"
 
 module Rube
   module Marshal
@@ -213,10 +214,46 @@ module Rube
         assert_equal ["Evil#_load"], result.sinks.map { |s| "#{s.class_name}##{s.sink_method}" }
       end
 
+      def tripwires(blob)
+        parse(blob).sinks.select { |node| node.class_name == AdversarialCorpus::TRIPWIRE_CLASS }
+      end
+
       def test_class_name_node_is_traversable
         result = parse(::Marshal.dump(Fixture.new))
-        symbols = result.nodes.select { |n| n.type == :symbol }
-        refute_empty symbols
+        names = result.nodes.select { |node| node.type == :symbol }.map(&:value)
+        assert_includes names, :"Rube::Marshal::ParserTest::Fixture"
+      end
+
+      def test_class_name_slot_gadget_is_reachable_in_every_slot
+        blind = AdversarialCorpus::CLASS_NAME_SLOTS.reject { |_slot, bytes| tripwires(bytes).any? }
+
+        assert_empty blind.keys,
+                     "class-name node discarded, hiding a sink, in: #{blind.keys.join(', ')}"
+      end
+
+      def test_class_name_slot_gadget_is_counted_once_per_slot
+        duplicated = AdversarialCorpus::CLASS_NAME_SLOTS.select { |_slot, bytes| tripwires(bytes).length > 1 }
+
+        assert_empty duplicated.keys, "a node must be traversed exactly once"
+      end
+
+      def test_distinct_instance_variable_names_reach_the_sink
+        assert_equal 1, tripwires(AdversarialCorpus::IVAR_DISTINCT_NAMES_CONTROL).length,
+                     "control failed, so the collision tests below would prove nothing"
+      end
+
+      def test_duplicate_instance_variable_names_do_not_delete_a_sink
+        blind = AdversarialCorpus::IVAR_COLLISION_SHAPES.reject { |_shape, bytes| tripwires(bytes).any? }
+
+        assert_empty blind.keys,
+                     "duplicate ivar name deleted a subtree in: #{blind.keys.join(', ')}"
+      end
+
+      def test_duplicate_instance_variable_names_retain_both_values
+        AdversarialCorpus::IVAR_COLLISION_SHAPES.each do |shape, bytes|
+          nils = parse(bytes).nodes.count { |node| node.type == :nil }
+          assert_equal 2, nils, "#{shape} lost an instance variable value node"
+        end
       end
 
       def test_rejects_depth_beyond_limit

PROJECTS/beginner/deserialization-gadget-lab/test/support/adversarial_corpus.rb

@@ -35,6 +35,10 @@ module Rube
       ":#{fixnum(name.bytesize)}#{name}".b
     end
 
+    def symlink(index)
+      ";#{fixnum(index)}".b
+    end
+
     def str(text)
       "\"#{fixnum(text.bytesize)}#{text}".b
     end
@@ -43,8 +47,9 @@ module Rube
       "#{HEADER}#{body}".b
     end
 
-    def entry(name, bytes, verdict, rationale)
-      { name: name, bytes: bytes.b, verdict: verdict, rationale: rationale }.freeze
+    def entry(name, bytes, verdict, rationale, allowed: [])
+      { name: name, bytes: bytes.b, verdict: verdict, rationale: rationale,
+        allowed: allowed.freeze }.freeze
     end
 
     NESTED_DEPTH = 80
@@ -52,6 +57,44 @@ module Rube
     MANY_SYMBOLS = 400
     HUGE_SCALAR = 300_000
 
+    TRIPWIRE_CLASS = "Tripwire"
+    HOST_CLASS = "Comparable"
+    OBJECT_CLASS = "Foo"
+    COLLIDING_IVAR = "@a"
+    HIDDEN_IVAR = "@z"
+
+    TRIPWIRE = "U#{sym(TRIPWIRE_CLASS)}0".b
+
+    CLASS_NAME_GADGET = "I#{sym(HOST_CLASS)}#{fixnum(1)}#{sym(HIDDEN_IVAR)}#{TRIPWIRE}".b
+
+    CLASS_NAME_SLOTS = {
+      object: stream("o#{CLASS_NAME_GADGET}#{fixnum(0)}"),
+      struct: stream("S#{CLASS_NAME_GADGET}#{fixnum(0)}"),
+      userdef: stream("u#{CLASS_NAME_GADGET}#{fixnum(1)}x"),
+      usermarshal: stream("U#{CLASS_NAME_GADGET}0"),
+      data: stream("d#{CLASS_NAME_GADGET}0"),
+      user_class: stream("C#{CLASS_NAME_GADGET}#{str('body')}"),
+      extended: stream("e#{CLASS_NAME_GADGET}#{str('body')}")
+    }.freeze
+
+    CLASS_NAME_SLOTS_WITH_NON_SINK_HOST = %i[object struct user_class extended].freeze
+
+    IVAR_COLLISION_SHAPES = {
+      wrapper_symlink_name:
+        stream("I#{str('hello')}#{fixnum(2)}#{sym(COLLIDING_IVAR)}#{TRIPWIRE}#{symlink(0)}0"),
+      wrapper_redefined_name:
+        stream("I#{str('hello')}#{fixnum(2)}#{sym(COLLIDING_IVAR)}#{TRIPWIRE}#{sym(COLLIDING_IVAR)}0"),
+      object_symlink_name:
+        stream("o#{sym(OBJECT_CLASS)}#{fixnum(2)}#{sym(COLLIDING_IVAR)}#{TRIPWIRE}#{symlink(1)}0"),
+      object_redefined_name:
+        stream("o#{sym(OBJECT_CLASS)}#{fixnum(2)}#{sym(COLLIDING_IVAR)}#{TRIPWIRE}#{sym(COLLIDING_IVAR)}0")
+    }.freeze
+
+    IVAR_COLLISION_SHAPES_INSIDE_OBJECT = %i[object_symlink_name object_redefined_name].freeze
+
+    IVAR_DISTINCT_NAMES_CONTROL =
+      stream("I#{str('hello')}#{fixnum(2)}#{sym(COLLIDING_IVAR)}#{TRIPWIRE}#{sym(HIDDEN_IVAR)}0")
+
     CASES = [
       entry(:nil_literal, stream("0"), VERDICT_ACCEPT,
             "the smallest legal stream"),
@@ -136,7 +179,35 @@ module Rube
             "a sink hidden where a symbol is expected must not disappear from the report"),
       entry(:plain_object_no_sink, stream("o#{sym('ERB')}\x06#{sym('@src')}#{str('payload')}"),
             VERDICT_REJECT,
-            "carries no sink tag at all, which is exactly why sink detection is insufficient")
+            "carries no sink tag at all, which is exactly why sink detection is insufficient"),
+
+      entry(:ivar_collision_wrapper_symlink_name,
+            IVAR_COLLISION_SHAPES[:wrapper_symlink_name], VERDICT_REJECT,
+            "two ivars share a name under I, so a Hash write must not delete the first value node"),
+      entry(:ivar_collision_wrapper_redefined_name,
+            IVAR_COLLISION_SHAPES[:wrapper_redefined_name], VERDICT_REJECT,
+            "the same collision spelled as a second symbol definition rather than a symlink"),
+      entry(:ivar_collision_object_symlink_name,
+            IVAR_COLLISION_SHAPES[:object_symlink_name], VERDICT_REJECT,
+            "the collision inside an object body, where the allowlist would otherwise pass it",
+            allowed: [OBJECT_CLASS]),
+      entry(:ivar_collision_object_redefined_name,
+            IVAR_COLLISION_SHAPES[:object_redefined_name], VERDICT_REJECT,
+            "object body collision spelled as a redefinition",
+            allowed: [OBJECT_CLASS]),
+
+      entry(:class_name_slot_object, CLASS_NAME_SLOTS[:object], VERDICT_REJECT,
+            "control: the one slot that already retains its class-name node catches this gadget",
+            allowed: [HOST_CLASS]),
+      entry(:class_name_slot_struct, CLASS_NAME_SLOTS[:struct], VERDICT_REJECT,
+            "byte-identical gadget in a struct class-name slot",
+            allowed: [HOST_CLASS]),
+      entry(:class_name_slot_user_class, CLASS_NAME_SLOTS[:user_class], VERDICT_REJECT,
+            "byte-identical gadget in a user-class wrapper slot",
+            allowed: [HOST_CLASS]),
+      entry(:class_name_slot_extended, CLASS_NAME_SLOTS[:extended], VERDICT_REJECT,
+            "byte-identical gadget in an extend wrapper slot",
+            allowed: [HOST_CLASS])
     ].freeze
   end
 end