SECURITY FIX: Do not allow GET access to JSON views without including…

… CSRF token. See http://haacked.com/archive/2009/06/25/json-hijacking.aspx
railsdog · Oct 30, 2010 · d881b2b · d881b2b
1 parent 19944bd
commit d881b2b
Show file tree

Hide file tree

Showing 9 changed files with 23 additions and 13 deletions.
diff --git a/app/controllers/admin/overview_controller.rb b/app/controllers/admin/overview_controller.rb
@@ -8,7 +8,7 @@ def index
     #@users = User.find_with_deleted(:all, :order => 'updated_at desc')
 #  going to list today's orders, yesterday's orders, older orders
 # have a filter / search at the top
-    # @orders, @ 
+    # @orders, @
   end
 
 end
diff --git a/app/controllers/admin/products_controller.rb b/app/controllers/admin/products_controller.rb
@@ -1,5 +1,6 @@
 class Admin::ProductsController < Admin::BaseController
   resource_controller
+  before_filter :check_json_authenticity, :only => :index
   before_filter :load_data, :except => :index
 
   index.response do |wants|

diff --git a/app/controllers/admin/users_controller.rb b/app/controllers/admin/users_controller.rb
@@ -1,5 +1,6 @@
 class Admin::UsersController < Admin::BaseController
   resource_controller
+  before_filter :check_json_authenticity, :only => :index
   before_filter :load_roles, :only => [:edit, :new, :update, :create]
 
   create.after :save_user_roles
@@ -9,7 +10,7 @@ class Admin::UsersController < Admin::BaseController
     wants.html { render :action => :index }
     wants.json { render :json => @collection.to_json(:include => {:bill_address => {:include => [:state, :country]}, :ship_address => {:include => [:state, :country]}}) }
   end
-  
+
   destroy.success.wants.js { render_js_for_destroy }
 
   private

diff --git a/app/controllers/spree/base_controller.rb b/app/controllers/spree/base_controller.rb
@@ -59,14 +59,20 @@ def title
 
   protected
 
+  # Index request for JSON needs to pass a CSRF token in order to prevent JSON Hijacking
+  def check_json_authenticity
+    return unless request.format.js? or request.format.json?
+    form_authenticity_token == params[request_forgery_protection_token] || raise(ActionController::InvalidAuthenticityToken)
+  end
+
   def default_title
     Spree::Config[:site_name]
   end
-  
+
   def accurate_title
     return nil
   end
-  
+
   def reject_unknown_object
     # workaround to catch problems with loading errors for permalink ids (reconsider RC permalink hack elsewhere?)
     begin

diff --git a/vendor/extensions/overview_dashboard/app/controllers/admin/overview_controller.rb b/vendor/extensions/overview_dashboard/app/controllers/admin/overview_controller.rb
@@ -1,6 +1,8 @@
 # this clas was inspired (heavily) from the mephisto admin architecture
 
 class Admin::OverviewController < Admin::BaseController
+  before_filter :check_json_authenticity, :only => :get_report_data
+
   #todo, add rss feed of information that is happening
 
   def index
@@ -57,7 +59,7 @@ def conditions(params)
         ["completed_at >= ?", params[:from]]
       end
   end
-  
+
   def fill_empty_entries(orders, params)
     from_date = params[:from].to_date
     to_date = (params[:to] || Time.now).to_date

diff --git a/vendor/extensions/overview_dashboard/public/javascripts/dashboard.js b/vendor/extensions/overview_dashboard/public/javascripts/dashboard.js
@@ -95,15 +95,15 @@ jQuery(document).ready(function(){
 
 		jQuery.ajax({
 	       type: 'GET',
-	       url: 'admin/overview/get_report_data',
-	       data: ({report: 'orders_by_day', name: report, value: value, authenticity_token: AUTH_TOKEN}),
+	       url: 'admin/overview/get_report_data?authenticity_token=' + AUTH_TOKEN,
+	       data: ({report: 'orders_by_day', name: report, value: value}),
 	       success: handle_orders_by_day
 		});
 
 		jQuery.ajax({
 	       type: 'GET',
-	       url: 'admin/overview/get_report_data',
-	       data: ({report: 'orders_totals', name: report, authenticity_token: AUTH_TOKEN}),
+	       url: 'admin/overview/get_report_data?authenticity_token=' + AUTH_TOKEN,
+	       data: ({report: 'orders_totals', name: report}),
 	       success: handle_orders_total
 		});
 

diff --git a/vendor/extensions/theme_default/app/views/shared/_admin_head.html.erb b/vendor/extensions/theme_default/app/views/shared/_admin_head.html.erb
@@ -8,7 +8,7 @@
 <%= javascript_include_tag('jquery.template') unless controller.controller_name == "overview" %>
 <%= javascript_include_tag 'spree', 'nested-attribute', 'zone', 'calculator', 'gateway' %>
 <%= javascript_tag "$ = jQuery;" %>
-<%= javascript_tag "var AUTH_TOKEN = #{form_authenticity_token.inspect};" if protect_against_forgery? %>
+<%= javascript_tag "var AUTH_TOKEN = encodeURIComponent(#{form_authenticity_token.inspect});" %>
 <%= javascript_include_tag 'jquery.alerts/jquery.alerts.js' %>
 <%= javascript_include_tag 'jquery.autocomplete.min.js' %>
 
@@ -17,5 +17,5 @@
 <%= stylesheet_link_tag 'jquery.autocomplete.css' %>
 
 <%= javascript_tag "jQuery.alerts.dialogClass = 'spree';" %>
-<%= unobtrusive_datepicker_includes %>	
+<%= unobtrusive_datepicker_includes %>
 <%= yield :head %>
diff --git a/vendor/extensions/theme_default/public/javascripts/admin/checkouts/edit.js b/vendor/extensions/theme_default/public/javascripts/admin/checkouts/edit.js
@@ -40,7 +40,7 @@ jQuery(document).ready(function(){
     });
   }
 
-  $("#customer_search").autocomplete("/admin/users.json", {
+  $("#customer_search").autocomplete("/admin/users.json?authenticity_token=" + AUTH_TOKEN, {
     minChars: 5,
     delay: 1500,
     parse: prep_autocomplete_data,

diff --git a/vendor/extensions/theme_default/public/javascripts/admin/orders/edit.js b/vendor/extensions/theme_default/public/javascripts/admin/orders/edit.js
@@ -75,7 +75,7 @@ jQuery(document).ready(function(){
     });
   }
 
-  $("#add_product_name").autocomplete("/admin/products.json", {
+  $("#add_product_name").autocomplete("/admin/products.json?authenticity_token=" + AUTH_TOKEN, {
       parse: prep_autocomplete_data,
       formatItem: function(item) {
         return format_autocomplete(item);