CVE-2024-28121: Insufficient Information (NVD-CWE-noinfo)
value: stimulus_reflex is a system to extend the capabilities of both Rails and Stimulus by intercepting user interactions and passing them to Rails over real-time websockets. In affected versions more methods than expected can be called on reflex instances. Being able to call some of them has security implications. To invoke a reflex a websocket message of the following shape is sent: `\"target\":\"[class_name]#[method_name]\",\"args\":[]`. The server will proceed to instantiate `reflex` using the provided `class_name` as long as it extends `StimulusReflex::Reflex`. It then attempts to call `method_name` on the instance with the provided arguments. This is problematic as `reflex.method method_name` can be more methods that those explicitly specified by the developer in their reflex class. A good example is the instance_variable_set method. This vulnerability has been patched in versions 3.4.2 and 3.5.0.rc4. Users unable to upgrade should: see the backing GHSA advisory for mitigation advice.
CWE description: There is insufficient information about the issue to classify it; details are unkown or unspecified.
Published: 2024-03-12T20:15Z
Repository: https://github.com/stimulusreflex/stimulus_reflex
Language: Ruby
Files changed: lib/stimulus_reflex/reflex_factory.rb, test/reflex_factory_test.rb, test/test_helper.rb
Commit date: 2024-03-12 06:55:07 +0100
Commit message: Merge pull request from GHSA-f78j-4w3g-4q65

* Validate Reflex `method_name` to  only allow safe reflex method-calls

* Only allow public instance methods

* Allow reflex methods from concerns to be called
Diff: diff --git a/lib/stimulus_reflex/reflex_factory.rb b/lib/stimulus_reflex/reflex_factory.rb
index f587ba6..8c77101 100644
--- a/lib/stimulus_reflex/reflex_factory.rb
+++ b/lib/stimulus_reflex/reflex_factory.rb
@@ -1,26 +1,59 @@
 # frozen_string_literal: true
 
 class StimulusReflex::ReflexFactory
   attr_reader :channel, :data
 
-  delegate :reflex_name, to: :data
+  delegate :reflex_name, :method_name, to: :data
 
   def initialize(channel, data)
     @channel = channel
     @data = StimulusReflex::ReflexData.new(data)
   end
 
   def call
+    verify_method_name!
     reflex_class.new(channel, reflex_data: data)
   end
 
   private
 
+  def verify_method_name!
+    return if default_reflex?
+
+    argument_error = ArgumentError.new("Reflex method '#{method_name}' is not defined on class '#{reflex_name}' or on any of its ancestors")
+
+    if reflex_method.nil?
+      raise argument_error
+    end
+
+    if !safe_ancestors.include?(reflex_method.owner)
+      raise argument_error
+    end
+  end
+
   def reflex_class
-    reflex_name.constantize.tap do |klass|
+    @reflex_class ||= reflex_name.constantize.tap do |klass|
       unless klass.ancestors.include?(StimulusReflex::Reflex)
         raise ArgumentError.new("#{reflex_name} is not a StimulusReflex::Reflex")
       end
     end
   end
+
+  def reflex_method
+    if reflex_class.public_instance_methods.include?(method_name.to_sym)
+      reflex_class.public_instance_method(method_name)
+    end
+  end
+
+  def default_reflex?
+    method_name == "default_reflex" && reflex_method.owner == ::StimulusReflex::Reflex
+  end
+
+  def safe_ancestors
+    # We want to include every class and module up to the `StimulusReflex::Reflex` class,
+    # but not the StimulusReflex::Reflex itself
+    reflex_class_index = reflex_class.ancestors.index(StimulusReflex::Reflex) - 1
+
+    reflex_class.ancestors.to(reflex_class_index)
+  end
 end
diff --git a/test/reflex_factory_test.rb b/test/reflex_factory_test.rb
new file mode 100644
index 0000000..4241326
--- /dev/null
+++ b/test/reflex_factory_test.rb
@@ -0,0 +1,79 @@
+# frozen_string_literal: true
+
+require_relative "test_helper"
+
+class StimulusReflex::ReflexFactoryTest < ActionCable::Channel::TestCase
+  tests StimulusReflex::Channel
+
+  test "reflex class needs to be an ancestor of StimulusReflex::Reflex" do
+    exception = assert_raises(NameError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "Object#inspect"}).call }
+    assert_equal "uninitialized constant ObjectReflex", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "NoReflex#no_reflex"}).call }
+    assert_equal "NoReflex is not a StimulusReflex::Reflex", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "No#no_reflex"}).call }
+    assert_equal "NoReflex is not a StimulusReflex::Reflex", exception.message
+  end
+
+  test "doesn't raise if owner of method is ancestor of reflex class and descendant of StimulusReflex::Reflex" do
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "ApplicationReflex#default_reflex"}).call }
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "ApplicationReflex#application_reflex"}).call }
+
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#default_reflex"}).call }
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#application_reflex"}).call }
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#post_reflex"}).call }
+
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#default_reflex"}).call }
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#application_reflex"}).call }
+    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#increment"}).call }
+  end
+
+  test "raises if method is not owned by a descendant of StimulusReflex::Reflex" do
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#itself"}).call }
+    assert_equal "Reflex method 'itself' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#itself"}).call }
+    assert_equal "Reflex method 'itself' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#itself"}).call }
+    assert_equal "Reflex method 'itself' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#binding"}).call }
+    assert_equal "Reflex method 'binding' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#byebug"}).call }
+    assert_equal "Reflex method 'byebug' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#debug"}).call }
+    assert_equal "Reflex method 'debug' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#post_reflex"}).call }
+    assert_equal "Reflex method 'post_reflex' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message
+  end
+
+  test "raises if method is a private method" do
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#private_application_reflex"}).call }
+    assert_equal "Reflex method 'private_application_reflex' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#private_application_reflex"}).call }
+    assert_equal "Reflex method 'private_application_reflex' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#private_post_reflex"}).call }
+    assert_equal "Reflex method 'private_post_reflex' is not defined on class 'PostReflex' or on any of its ancestors", exception.message
+
+    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "CounterReflex#private_post_reflex"}).call }
+    assert_equal "Reflex method 'private_post_reflex' is not defined on class 'CounterReflex' or on any of its ancestors", exception.message
+  end
+
+  test "safe_ancestors" do
+    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#default_reflex"})
+    assert_equal [ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)
+
+    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#default_reflex"})
+    assert_equal [PostReflex, ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)
+
+    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "CounterReflex#increment"})
+    assert_equal [CounterReflex, CounterConcern, ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)
+  end
+end
diff --git a/test/test_helper.rb b/test/test_helper.rb
index de9b5ac..a9d7e63 100644
--- a/test/test_helper.rb
+++ b/test/test_helper.rb
@@ -48,10 +48,44 @@ class TestModel
   def to_gid_param
     "xxxyyyzzz"
   end
 end
 
+class ApplicationReflex < StimulusReflex::Reflex
+  def application_reflex
+  end
+
+  private
+
+  def private_application_reflex
+  end
+end
+
+class PostReflex < ApplicationReflex
+  def post_reflex
+  end
+
+  private
+
+  def private_post_reflex
+  end
+end
+
+class NoReflex
+  def no_reflex
+  end
+end
+
+module CounterConcern
+  def increment
+  end
+end
+
+class CounterReflex < ApplicationReflex
+  include CounterConcern
+end
+
 module ActionCable
   module Channel
     class ConnectionStub
       def connection_identifier
         connection_gid identifiers.map { |id| send(id.to_sym) if id }.compact

Vulnerable code:   delegate :reflex_name, to: :data
    reflex_name.constantize.tap do |klass|
Fixed code:   delegate :reflex_name, :method_name, to: :data
    verify_method_name!
  def verify_method_name!
    return if default_reflex?

    argument_error = ArgumentError.new("Reflex method '#{method_name}' is not defined on class '#{reflex_name}' or on any of its ancestors")

    if reflex_method.nil?
      raise argument_error
    end

    if !safe_ancestors.include?(reflex_method.owner)
      raise argument_error
    end
  end

    @reflex_class ||= reflex_name.constantize.tap do |klass|

  def reflex_method
    if reflex_class.public_instance_methods.include?(method_name.to_sym)
      reflex_class.public_instance_method(method_name)
    end
  end

  def default_reflex?
    method_name == "default_reflex" && reflex_method.owner == ::StimulusReflex::Reflex
  end

  def safe_ancestors
    # We want to include every class and module up to the `StimulusReflex::Reflex` class,
    # but not the StimulusReflex::Reflex itself
    reflex_class_index = reflex_class.ancestors.index(StimulusReflex::Reflex) - 1

    reflex_class.ancestors.to(reflex_class_index)
  end
# frozen_string_literal: true

require_relative "test_helper"

class StimulusReflex::ReflexFactoryTest < ActionCable::Channel::TestCase
  tests StimulusReflex::Channel

  test "reflex class needs to be an ancestor of StimulusReflex::Reflex" do
    exception = assert_raises(NameError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "Object#inspect"}).call }
    assert_equal "uninitialized constant ObjectReflex", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "NoReflex#no_reflex"}).call }
    assert_equal "NoReflex is not a StimulusReflex::Reflex", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "No#no_reflex"}).call }
    assert_equal "NoReflex is not a StimulusReflex::Reflex", exception.message
  end

  test "doesn't raise if owner of method is ancestor of reflex class and descendant of StimulusReflex::Reflex" do
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "ApplicationReflex#default_reflex"}).call }
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "ApplicationReflex#application_reflex"}).call }

    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#default_reflex"}).call }
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#application_reflex"}).call }
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "PostReflex#post_reflex"}).call }

    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#default_reflex"}).call }
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#application_reflex"}).call }
    assert_nothing_raised { StimulusReflex::ReflexFactory.new(subscribe, {version: StimulusReflex::VERSION, target: "CounterReflex#increment"}).call }
  end

  test "raises if method is not owned by a descendant of StimulusReflex::Reflex" do
    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#itself"}).call }
    assert_equal "Reflex method 'itself' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#itself"}).call }
    assert_equal "Reflex method 'itself' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#itself"}).call }
    assert_equal "Reflex method 'itself' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#binding"}).call }
    assert_equal "Reflex method 'binding' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#byebug"}).call }
    assert_equal "Reflex method 'byebug' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#debug"}).call }
    assert_equal "Reflex method 'debug' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#post_reflex"}).call }
    assert_equal "Reflex method 'post_reflex' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message
  end

  test "raises if method is a private method" do
    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#private_application_reflex"}).call }
    assert_equal "Reflex method 'private_application_reflex' is not defined on class 'ApplicationReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#private_application_reflex"}).call }
    assert_equal "Reflex method 'private_application_reflex' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#private_post_reflex"}).call }
    assert_equal "Reflex method 'private_post_reflex' is not defined on class 'PostReflex' or on any of its ancestors", exception.message

    exception = assert_raises(ArgumentError) { StimulusReflex::ReflexFactory.new(subscribe, {target: "CounterReflex#private_post_reflex"}).call }
    assert_equal "Reflex method 'private_post_reflex' is not defined on class 'CounterReflex' or on any of its ancestors", exception.message
  end

  test "safe_ancestors" do
    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "ApplicationReflex#default_reflex"})
    assert_equal [ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)

    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "PostReflex#default_reflex"})
    assert_equal [PostReflex, ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)

    reflex_factory = StimulusReflex::ReflexFactory.new(subscribe, {target: "CounterReflex#increment"})
    assert_equal [CounterReflex, CounterConcern, ApplicationReflex, StimulusReflex::CableReadiness], reflex_factory.send(:safe_ancestors)
  end
end
class ApplicationReflex < StimulusReflex::Reflex
  def application_reflex
  end

  private

  def private_application_reflex
  end
end

class PostReflex < ApplicationReflex
  def post_reflex
  end

  private

  def private_post_reflex
  end
end

class NoReflex
  def no_reflex
  end
end

module CounterConcern
  def increment
  end
end

class CounterReflex < ApplicationReflex
  include CounterConcern
end