fixes #2631 - fix remote code execution via controller name (CVE-2013-2121)

(cherry picked from commit ef4b97d1)

app/controllers/application_controller.rb

@@ -141,7 +141,7 @@ class ApplicationController < ActionController::Base
     obj = controller_name.singularize
     # determine if we are searching for a numerical id or plain name
     cond = "find_by_" + ((id =~ /^\d+$/ && (id=id.to_i)) ? "id" : "name")
-    not_found and return unless eval("@#{obj} = #{obj.camelize}.#{cond}(id)")
+    not_found and return unless instance_variable_set("@#{obj}", obj.camelize.constantize.send(cond, id))
   end
 
   def notice notice
@@ -232,10 +232,10 @@ class ApplicationController < ActionController::Base
   end
 
   def process_success hash = {}
-    hash[:object]                 ||= eval("@#{controller_name.singularize}")
+    hash[:object]                 ||= instance_variable_get("@#{controller_name.singularize}")
     hash[:object_name]            ||= hash[:object].to_s
     hash[:success_msg]            ||= "Successfully #{action_name.pluralize.sub(/es$/,"ed").sub(/ys$/, "yed")} #{hash[:object_name]}."
-    hash[:success_redirect]       ||= eval("#{controller_name}_url")
+    hash[:success_redirect]       ||= send("#{controller_name}_url")
     hash[:json_code]                = :created if action_name == "create"
 
     return render :json => {:redirect => hash[:success_redirect]} if hash[:redirect_xhr]
@@ -250,13 +250,13 @@ class ApplicationController < ActionController::Base
   end
 
   def process_error hash = {}
-    hash[:object] ||= eval("@#{controller_name.singularize}")
+    hash[:object] ||= instance_variable_get("@#{controller_name.singularize}")
 
     case action_name
     when "create" then hash[:render] ||= "new"
     when "update" then hash[:render] ||= "edit"
     else
-      hash[:redirect] ||= eval("#{controller_name}_url")
+      hash[:redirect] ||= send("#{controller_name}_url")
     end
 
     hash[:json_code] ||= :unprocessable_entity

app/controllers/bookmarks_controller.rb

@@ -28,7 +28,7 @@ class BookmarksController < ApplicationController
 
     respond_to do |format|
       if @bookmark.save
-        format.html { redirect_to(eval(@bookmark.controller+"_path"), :notice => 'Bookmark was successfully created.') }
+        format.html { redirect_to(send("#{@bookmark.controller}_path"), :notice => 'Bookmark was successfully created.') }
       else
         format.html { render :action => "new" }
       end

app/controllers/unattended_controller.rb

@@ -180,7 +180,7 @@ class UnattendedController < ApplicationController
 
   def load_template_vars
     # load the os family default variables
-    eval "#{@host.os.pxe_type}_attributes"
+    send "#{@host.os.pxe_type}_attributes"
   end
 
   def jumpstart_attributes

app/helpers/application_helper.rb

@@ -167,7 +167,7 @@ module ApplicationHelper
   end
 
   def auto_complete_search(method, val,tag_options = {}, completion_options = {})
-    path = eval("#{controller_name}_path")
+    path = send("#{controller_name}_path")
     options = tag_options.merge(:class => "auto_complete_input")
     text_field_tag(method, val, options) + auto_complete_clear_value_button(method) +
       auto_complete_field_jquery(method, "#{path}/auto_complete_#{method}", completion_options)
@@ -178,7 +178,7 @@ module ApplicationHelper
   end
 
   def method_path method
-    eval("#{method}_#{controller_name}_path")
+    send("#{method}_#{controller_name}_path")
   end
 
   def edit_textfield(object, property, options={})

app/helpers/audits_helper.rb

@@ -11,8 +11,7 @@ module AuditsHelper
       when 'last_login_on'
         change.to_s(:short)
       when /.*_id$/
-        model = (eval name.humanize)
-        model.find(change).to_label
+        name.humanize.constantize.find(change).to_label
       else
         change.to_s
     end.truncate(50)

app/helpers/common_parameters_helper.rb

@@ -4,7 +4,7 @@ module CommonParametersHelper
     return true if authorized_for(controller, action)
 
     operation = "#{action}_my_#{controller.singularize}".to_sym
-    User.current.allowed_to?(operation) and User.current.send(controller).include?(eval("@#{controller.singularize}"))
+    User.current.allowed_to?(operation) and User.current.send(controller).include?(instance_variable_get("@#{controller.singularize}"))
   end
 
   def parameters_title

app/helpers/home_helper.rb

@@ -78,7 +78,7 @@ module HomeHelper
   end
 
   def menu(tab, myBookmarks ,path = nil)
-    path ||= eval("hash_for_#{tab}_path")
+    path ||= send("hash_for_#{tab}_path")
     return '' unless authorized_for(path[:controller], path[:action] )
     b = myBookmarks.map{|b| b if b.controller == path[:controller]}.compact
     out = content_tag :li, :id => "menu_tab_#{tab}" do
@@ -99,7 +99,7 @@ module HomeHelper
   def allowed_choices choices, action = "index"
     choices.map do |opt|
       name, kontroller = opt
-      url = eval("#{kontroller}_url")
+      url = send("#{kontroller}_url")
       authorized_for(kontroller, action) ? [name, url] : nil
     end.compact.sort
   end

app/helpers/layout_helper.rb

@@ -141,7 +141,7 @@ module LayoutHelper
   end
 
   def submit_or_cancel f, overwrite = false, args = { }
-    args[:cancel_path] ||= eval "#{controller_name}_path"
+    args[:cancel_path] ||= send("#{controller_name}_path")
     content_tag(:div, :class => "form-actions") do
       text    = overwrite ? "Overwrite" : "Submit"
       options = overwrite ? {:class => "btn btn-danger"} : {:class => "btn btn-primary"}

app/models/bookmark.rb

@@ -5,6 +5,9 @@ class Bookmark < ActiveRecord::Base
   validates_uniqueness_of :name, :unless => Proc.new{|b| Bookmark.my_bookmarks.where(:name => b.name).empty?}
   validates_presence_of :name, :controller, :query
   validates_format_of :controller, :with => /\A(\S+)\Z/, :message => "can't be blank or contain white spaces."
+  validates :controller, :inclusion => {
+                         :in => ["dashboard"] + ActiveRecord::Base.connection.tables.map(&:to_s),
+                         :message => "%{value} is not a valid controller" }
   default_scope :order => :name
   before_validation :set_default_user
 

app/views/bookmarks/_list.html.erb

 <% if bookmarks.any? -%>
 <ul class='dropdown-menu'>
   <% bookmarks.each do |bookmark| -%>
-      <li><%= link_to_if_authorized bookmark.name, eval("hash_for_#{bookmark.controller}_path").merge(:search => bookmark.query) %></li>
+      <li><%= link_to_if_authorized bookmark.name, send("hash_for_#{bookmark.controller}_path").merge(:search => bookmark.query) %></li>
   <% end -%>
 </ul>
 <% end -%>

app/views/common/_puppetclasses_or_envs_changed.html.erb

@@ -29,4 +29,4 @@
     <%= link_to "Cancel","/" + controller_path, :class => "btn" %>
     <%= submit_tag "Update", :class => "btn btn-primary" %>
   </div>
-<% end -%>
\ No newline at end of file
+<% end -%>

app/views/common/_searchbar.erb

-<%= form_tag eval("#{controller_name}_path"), :method => "get", :class=>"form-inline form-search row-fluid" do %>
+<%= form_tag send("#{controller_name}_path"), :method => "get", :class=>"form-inline form-search row-fluid" do %>
     <div class="btn-toolbar btn-toolbar-condensed">
     <div class="btn-group span12">
       <div class="span9">
@@ -14,7 +14,7 @@
       <ul class="dropdown-menu pull-right">
         <% bookmarks = Bookmark.my_bookmarks.controller(controller_name) %>
         <% bookmarks.each do |bookmark| -%>
-            <li><%= link_to_if_authorized bookmark.name, eval("hash_for_#{bookmark.controller}_path").merge(:search => bookmark.query) %></li>
+            <li><%= link_to_if_authorized bookmark.name, send("hash_for_#{bookmark.controller}_path").merge(:search => bookmark.query) %></li>
         <% end -%>
         <li class="divider"></li>
         <li><%= link_to_function 'Bookmark this search', "$('#bookmarks-modal').modal();",

lib/foreman/controller/host_details.rb

@@ -43,8 +43,8 @@ module Foreman::Controller::HostDetails
     @organization = params[:organization_id] ? Organization.find(params[:organization_id]) : nil
     @location = params[:location_id] ? Location.find(params[:location_id]) : nil
     Taxonomy.as_taxonomy @organization, @location do
-      if params["#{name}_id"].to_i > 0 and eval("@#{name} = #{name.capitalize}.find(params['#{name}_id'])")
-        item = eval("@#{controller_name.singularize} || #{controller_name.singularize.capitalize}.new(params[:#{controller_name.singularize}])")
+      if params["#{name}_id"].to_i > 0 and instance_variable_set("@#{name}",name.classify.constantize.find(params["#{name}_id"]))
+        item = instance_variable_get("@#{controller_name.singularize}") || controller_name.classify.constantize.new(params[controller_name.singularize])
         render :partial => root + name, :locals => { :item => item }
       else
         head(:not_found)
@@ -60,7 +60,7 @@ module Foreman::Controller::HostDetails
   # @host = Host.new params[:host]
   def item_object
     name = item_name
-    eval("@#{name} = #{name.capitalize}.new params[:#{name}]")
+    instance_variable_set("@#{name}", name.classify.constantize.new(params[name.to_sym]))
   end
 
 end

test/unit/bookmark_test.rb

@@ -20,6 +20,13 @@ class BookmarkTest < ActiveSupport::TestCase
     end
   end
 
+  test "validation fails when invalid controller name stored" do
+    b = Bookmark.create :name => "controller_test", :controller => "hosts", :query => "foo=bar", :public => true
+    assert b.valid?
+    b.controller = "foo bar"
+    assert !b.valid?
+  end
+
   private
 
   def enable_login &block