fix(marshalsea): a guard that asks the object what it is has already run the payload
LoadGuard#owner_name resolved the receiver with `receiver.is_a?(Module) ? receiver : receiver.class`. The receiver is the gadget. A method-erased proxy answers .class and .is_a? through method_missing, and method_missing is exactly what the shipped erb-def-module chain enters through, so identifying the object fired the chain it was about to veto. TracePoint does not trace a handler's own nested calls, so that detonation was invisible to the guard as well as unguarded by it. The observable result was a guard reporting a block on a payload that had already written its canary: strict guard: blocked -> deserialization hook (class with no name)#method_missing canary created? true "(class with no name)" was the tell: receiver.class had been answered by method_missing, which returned the anonymous Module that ERB#def_module builds. Resolve identity through Object#class, Object#is_a? and Module#name unbound and bind_call'd onto the receiver, so nothing dispatches to it. Both hook sets now veto with the canary absent and the real owner named. Side effect, and it is the stronger behaviour: Module#name read unbound means a class that overrides .name to raise is now named truthfully instead of reported anonymous. test_an_owner_that_refuses_to_name_itself_fails_closed asserted the old outcome and is rewritten to assert the new invariant. test_the_guard_never_dispatches_a_method_on_the_receiver_it_inspects pins it, and it isolates: instrumenting a wiped proxy shows [] against the fix and [:is_a?, :class] against the revert.
This commit is contained in:
parent
5f8096615e
commit
4166e60488
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 " \
|
||||
|
|
|
|||
Loading…
Reference in New Issue