Merge pull request #2729 from evazion/fix-notes-mass-assignment

Fix vuln allowing users to move notes between posts.

app/controllers/notes_controller.rb

@@ -20,7 +20,7 @@ class NotesController < ApplicationController
   end
 
   def create
-    @note = Note.create(params[:note])
+    @note = Note.create(create_params)
     respond_with(@note) do |fmt|
       fmt.json do
         if @note.errors.any?
@@ -34,7 +34,7 @@ class NotesController < ApplicationController
 
   def update
     @note = Note.find(params[:id])
-    @note.update_attributes(params[:note])
+    @note.update_attributes(update_params)
     respond_with(@note) do |format|
       format.json do
         if @note.errors.any?
@@ -60,6 +60,14 @@ class NotesController < ApplicationController
   end
 
 private
+  def update_params
+    params.require(:note).permit(:x, :y, :width, :height, :body)
+  end
+
+  def create_params
+    params.require(:note).permit(:x, :y, :width, :height, :body, :post_id)
+  end
+
   def pass_html_id
     if params[:note] && params[:note][:html_id]
       response.headers["X-Html-Id"] = params[:note][:html_id]

test/functional/notes_controller_test.rb

@@ -47,6 +47,13 @@ class NotesControllerTest < ActionController::TestCase
         @note.reload
         assert_equal("xyz", @note.body)
       end
+
+      should "not allow changing the post id to another post" do
+        @other = FactoryGirl.create(:post)
+        post :update, {:format => "json", :id => @note.id, :note => {:post_id => @other.id}}, {:user_id => @user.id}
+
+        assert_not_equal(@other.id, @note.reload.post_id)
+      end
     end
 
     context "destroy action" do