diff --git a/PROJECTS/beginner/deserialization-gadget-lab/lib/marshalsea/marshal/load_guard.rb b/PROJECTS/beginner/deserialization-gadget-lab/lib/marshalsea/marshal/load_guard.rb index 2c70f4f6..04300e12 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/lib/marshalsea/marshal/load_guard.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/lib/marshalsea/marshal/load_guard.rb @@ -19,6 +19,10 @@ module Marshalsea REASON = "deserialization hook %s#%s is not permitted" ANONYMOUS_OWNER = "(class with no name)" + CLASS_OF = ::Object.instance_method(:class).freeze + KIND_OF = ::Object.instance_method(:is_a?).freeze + NAME_OF = ::Module.instance_method(:name).freeze + LIMITATION_NOTICE = <<~NOTICE SECURITY LIMITATION @@ -109,8 +113,8 @@ module Marshalsea end def owner_name(receiver) - owner = receiver.is_a?(Module) ? receiver : receiver.class - name = owner.name + owner = KIND_OF.bind_call(receiver, ::Module) ? receiver : CLASS_OF.bind_call(receiver) + name = NAME_OF.bind_call(owner) name if name.is_a?(String) && !name.empty? rescue StandardError nil diff --git a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/load_guard_test.rb b/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/load_guard_test.rb index 89e2a20e..698872ec 100644 --- a/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/load_guard_test.rb +++ b/PROJECTS/beginner/deserialization-gadget-lab/test/marshal/load_guard_test.rb @@ -74,6 +74,8 @@ module Marshalsea def name_of(klass) = klass.name + def true_name_of(klass) = LoadGuard::NAME_OF.bind_call(klass) + def key_trigger_blob blob = ::Marshal.dump({ KeyTrigger.new => 1 }) BODIES.clear @@ -229,12 +231,14 @@ module Marshalsea end end - def test_an_owner_that_refuses_to_name_itself_fails_closed + def test_an_owner_that_refuses_to_name_itself_is_named_truthfully_anyway blob = ::Marshal.dump(HostileName.new) error = assert_raises(GuardedLoadError) { guard.load(blob) } - assert_includes error.message, LoadGuard::ANONYMOUS_OWNER, - "a class that raises from .name must not become permitted by accident" + assert_includes error.message, true_name_of(HostileName), + "the guard reads Module#name unbound, so a class that overrides .name " \ + "cannot control what the guard calls it" + assert_raises(NameError) { name_of(HostileName) } assert_empty BODIES end @@ -247,6 +251,43 @@ module Marshalsea assert_empty BODIES, "the placeholder must not be spellable as an allowlist entry" end + class DispatchTattle + DISPATCHED = [] + + instance_methods.each do |method_name| + undef_method(method_name) unless %i[__send__ __id__ object_id].include?(method_name) + end + + def marshal_dump = ["payload"] + + def respond_to_missing?(name, _include_private = false) = name == :marshal_load + + def method_missing(name, *args) + DISPATCHED << name + return BODIES << "DispatchTattle via method_missing" if name == :marshal_load + + super + end + end + + def test_the_guard_never_dispatches_a_method_on_the_receiver_it_inspects + DispatchTattle::DISPATCHED.clear + blob = ::Marshal.dump(DispatchTattle.new) + DispatchTattle::DISPATCHED.clear + BODIES.clear + + error = assert_raises(GuardedLoadError) { guard.load(blob) } + + assert_includes error.message, name_of(DispatchTattle), + "resolving the owner must still produce the real class name" + assert_empty BODIES, "the hook body must not run" + assert_empty DispatchTattle::DISPATCHED & %i[class is_a? name], + "identifying the receiver must not call a method ON the receiver. A " \ + "method-erased proxy answers .class and .is_a? through method_missing, " \ + "so a guard that asks the receiver what it is detonates the chain it " \ + "was about to veto, inside a TracePoint handler that does not trace itself" + end + def test_the_error_is_catchable_by_an_ordinary_rescue assert_operator GuardedLoadError, :<, StandardError, "SecurityError descends from Exception and would bypass every " \