From 5f8eb9fc8c13c80bdd33e811ccb15828a3165550 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jos=C3=A9=20Valim?= <jose.valim@gmail.com>
Date: Sun, 30 Aug 2009 13:11:40 +0200
Subject: [PATCH] Avoid arbitrary code executation by allowing dynamic tasks
 only dispatch to method missing cases [#4 status:resolved]

---
 lib/thor.rb            |  2 +-
 lib/thor/invocation.rb |  2 +-
 lib/thor/task.rb       | 20 +++++++++++++-------
 spec/task_spec.rb      | 14 ++++++++++----
 4 files changed, 25 insertions(+), 13 deletions(-)

diff --git a/lib/thor.rb b/lib/thor.rb
index 8dfcfd4c5..3b45c4e9b 100644
--- a/lib/thor.rb
+++ b/lib/thor.rb
@@ -135,7 +135,7 @@ def start(given_args=ARGV, config={})
           args, opts = given_args, {}
         end
 
-        task ||= Task.dynamic(meth)
+        task ||= Thor::Task::Dynamic.new(meth)
         trailing = args[Range.new(arguments.size, -1)]
         new(args, opts, config).invoke(task, trailing || [])
       end
diff --git a/lib/thor/invocation.rb b/lib/thor/invocation.rb
index c0388dd86..32e6a7245 100644
--- a/lib/thor/invocation.rb
+++ b/lib/thor/invocation.rb
@@ -153,7 +153,7 @@ def _validate_task(object, task) #:nodoc:
         raise "Expected Thor class, got #{klass}" unless klass <= Thor::Base
 
         task ||= klass.default_task if klass <= Thor
-        task = klass.all_tasks[task.to_s] || Task.dynamic(task) if task && !task.is_a?(Thor::Task)
+        task = klass.all_tasks[task.to_s] || Thor::Task::Dynamic.new(task) if task && !task.is_a?(Thor::Task)
         task
       end
 
diff --git a/lib/thor/task.rb b/lib/thor/task.rb
index 23d35b883..91c7564d3 100644
--- a/lib/thor/task.rb
+++ b/lib/thor/task.rb
@@ -1,11 +1,19 @@
 class Thor
   class Task < Struct.new(:name, :description, :usage, :options)
 
-    # Creates a dynamic task. Dynamic tasks are created on demand to allow method
-    # missing calls (since a method missing does not have a task object for it).
+    # A dynamic task that handles method missing scenarios.
     #
-    def self.dynamic(name)
-      new(name, "A dynamically-generated task", name.to_s)
+    class Dynamic < Task
+      def initialize(name)
+        super(name.to_s, "A dynamically-generated task", name.to_s)
+      end
+
+      def run(instance, args=[])
+        unless (instance.methods & [name.to_s, name.to_sym]).empty?
+          raise Error, "could not find Thor class or task '#{name}'"
+        end
+        super
+      end
     end
 
     def initialize(name, description, usage, options=nil)
@@ -37,8 +45,6 @@ def run(instance, args=[])
     # injected in the usage.
     #
     def formatted_usage(klass=nil, namespace=false, show_options=true)
-      formatted = ''
-
       formatted = if namespace.is_a?(String)
         "#{namespace}:"
       elsif klass && namespace
@@ -77,7 +83,7 @@ def formatted_options
       #
       def public_method?(instance) #:nodoc:
         collection = instance.private_methods + instance.protected_methods
-        !(collection).include?(name.to_s) && !(collection).include?(name.to_sym) # For Ruby 1.9
+        (collection & [name.to_s, name.to_sym]).empty?
       end
 
       # Clean everything that comes from the Thor gempath and remove the caller.
diff --git a/spec/task_spec.rb b/spec/task_spec.rb
index da0695dbe..ab7f5516c 100644
--- a/spec/task_spec.rb
+++ b/spec/task_spec.rb
@@ -41,10 +41,16 @@ def task(options={})
 
   describe "#dynamic" do
     it "creates a dynamic task with the given name" do
-      Thor::Task.dynamic('task').name.must == 'task'
-      Thor::Task.dynamic('task').description.must == 'A dynamically-generated task'
-      Thor::Task.dynamic('task').usage.must == 'task'
-      Thor::Task.dynamic('task').options.must == {}
+      Thor::Task::Dynamic.new('task').name.must == 'task'
+      Thor::Task::Dynamic.new('task').description.must == 'A dynamically-generated task'
+      Thor::Task::Dynamic.new('task').usage.must == 'task'
+      Thor::Task::Dynamic.new('task').options.must == {}
+    end
+
+    it "does not invoke an existing method" do
+      lambda {
+        Thor::Task::Dynamic.new('to_s').run([])
+      }.must raise_error(Thor::Error, "could not find Thor class or task 'to_s'")
     end
   end