FIX: 1868 Security: Dangerous Send

app/controllers/admin/site_settings_controller.rb

@@ -10,8 +10,8 @@ class Admin::SiteSettingsController < Admin::AdminController
     params.require(:id)
     id = params[:id]
     value = params[id]
-    StaffActionLogger.new(current_user).log_site_setting_change(id, SiteSetting.send(id), value) if SiteSetting.respond_to?(id)
-    SiteSetting.send("#{id}=", value)
+    StaffActionLogger.new(current_user).log_site_setting_change(id, SiteSetting.send(id), value) if SiteSetting.has_setting?(id)
+    SiteSetting.set(id, value)
     render nothing: true
   end
 

lib/site_setting_extension.rb

@@ -200,6 +200,18 @@ module SiteSettingExtension
     @last_message_sent = MessageBus.publish('/site_settings', {process: process_id})
   end
 
+  def has_setting?(name)
+    defaults.has_key?(name.to_sym) || defaults.has_key?("#{name}?".to_sym)
+  end
+
+  def set(name, value)
+    if has_setting?(name)
+      self.send("#{name}=", value)
+    else
+      raise ArgumentError.new("No setting named #{name} exists")
+    end
+  end
+
   protected
 
   def diff_hash(new_hash, old)

spec/components/site_setting_extension_spec.rb

@@ -58,6 +58,11 @@ describe SiteSettingExtension do
         settings.refresh!
         settings.test_setting.should_not == 77
       end
+
+      it "can be overridden with set" do
+        settings.set("test_setting", 12)
+        settings.test_setting.should == 12
+      end
     end
   end
 
@@ -89,6 +94,11 @@ describe SiteSettingExtension do
         settings.test_str = 100
         settings.test_str.should.eql? "100"
       end
+
+      it "can be overridden with set" do
+        settings.set("test_str", "hi")
+        settings.test_str.should == "hi"
+      end
     end
   end
 
@@ -128,6 +138,11 @@ describe SiteSettingExtension do
         settings.refresh!
         settings.test_hello?.should_not == false
       end
+
+      it "can be overridden with set" do
+        settings.set("test_hello", true)
+        settings.test_hello?.should == true
+      end
     end
   end
 
@@ -189,4 +204,14 @@ describe SiteSettingExtension do
     end
   end
 
+  describe "set for an invalid setting name" do
+    it "raises an error" do
+      settings.setting(:test_setting, 77)
+      settings.refresh!
+      expect {
+        settings.set("provider", "haxxed")
+      }.to raise_error(ArgumentError)
+    end
+  end
+
 end

spec/controllers/admin/site_settings_controller_spec.rb

@@ -25,6 +25,10 @@ describe Admin::SiteSettingsController do
 
     context 'update' do
 
+      before do
+        SiteSetting.setting(:test_setting, "default")
+      end
+
       it 'sets the value when the param is present' do
         SiteSetting.expects(:'test_setting=').with('hello').once
         xhr :put, :update, id: 'test_setting', test_setting: 'hello'
@@ -41,6 +45,12 @@ describe Admin::SiteSettingsController do
         StaffActionLogger.any_instance.expects(:log_site_setting_change).with('test_setting', 'previous', 'hello')
         xhr :put, :update, id: 'test_setting', test_setting: 'hello'
       end
+
+      it 'fails when a setting does not exist' do
+        expect {
+          xhr :put, :update, id: 'provider', provider: 'gotcha'
+        }.to raise_error(ArgumentError)
+      end
     end
 
   end