From 43ea87dffca2efbf17518cd2a7a2c911715b94e1 Mon Sep 17 00:00:00 2001 From: CarterPerez-dev Date: Tue, 28 Jul 2026 23:20:40 -0400 Subject: [PATCH] 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. --- .../deserialization-gadget-lab/justfile | 2 +- .../lib/rube/marshal/node.rb | 1 - .../lib/rube/marshal/parser.rb | 41 +++++----- .../test/corpus_test.rb | 47 ++++++++++- .../test/marshal/parser_test.rb | 41 +++++++++- .../test/support/adversarial_corpus.rb | 77 ++++++++++++++++++- 6 files changed, 180 insertions(+), 29 deletions(-) diff --git a/PROJECTS/beginner/deserialization-gadget-lab/justfile b/PROJECTS/beginner/deserialization-gadget-lab/justfile index b18789a3..068bfc58 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/justfile +++ b/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) }' diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb index eb51757a..cfd80bea 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/node.rb +++ b/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 diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb index 45138fb6..4e8dd949 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/rube/marshal/parser.rb +++ b/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 diff --git a/PROJECTS/beginner/deserialization-gadget-lab/test/corpus_test.rb b/PROJECTS/beginner/deserialization-gadget-lab/test/corpus_test.rb index 8189c9ac..0def6ce4 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/test/corpus_test.rb +++ b/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 diff --git a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb b/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb index 8d2d1463..a831c361 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/parser_test.rb +++ b/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 diff --git a/PROJECTS/beginner/deserialization-gadget-lab/test/support/adversarial_corpus.rb b/PROJECTS/beginner/deserialization-gadget-lab/test/support/adversarial_corpus.rb index 97d6b12a..c3c863db 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/test/support/adversarial_corpus.rb +++ b/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