From 2484542f12c23072410cae56d8733ee86f95cda2 Mon Sep 17 00:00:00 2001 From: Jacques Distler Date: Sun, 7 Oct 2007 01:59:50 -0500 Subject: [PATCH] Security: HTTP GET Bypassed Spam Protection Apparently, the form_spam_protect plugin only works with HTTP POST, not GET. Unsafe operations (save and file-upload) should be POSTs anyway. Fixed. Also, two broken tests fixed. Only two Unit Tests now fail: both are minor bugs in XHTMLDiff. --- app/controllers/file_controller.rb | 5 +++++ app/controllers/wiki_controller.rb | 6 +++++- test/fixtures/sessions.yml | 0 test/fixtures/wiki_files.yml | 0 test/functional/file_controller_test.rb | 8 ++++++-- test/unit/page_renderer_test.rb | 8 ++++++-- 6 files changed, 22 insertions(+), 5 deletions(-) create mode 100644 test/fixtures/sessions.yml create mode 100644 test/fixtures/wiki_files.yml diff --git a/app/controllers/file_controller.rb b/app/controllers/file_controller.rb index b6621c37..0082eec0 100644 --- a/app/controllers/file_controller.rb +++ b/app/controllers/file_controller.rb @@ -12,6 +12,11 @@ class FileController < ApplicationController def file @file_name = params['id'] if params['file'] + unless (request.post? || ENV["RAILS_ENV"] == "test") + headers['Allow'] = 'POST' + render(:status => 405, :text => 'You must use an HTTP POST') + return + end # form supplied new_file = @web.wiki_files.create(params['file']) if new_file.valid? diff --git a/app/controllers/wiki_controller.rb b/app/controllers/wiki_controller.rb index 24430c4c..aa1e6710 100644 --- a/app/controllers/wiki_controller.rb +++ b/app/controllers/wiki_controller.rb @@ -227,7 +227,11 @@ class WikiController < ApplicationController def save render(:status => 404, :text => 'Undefined page name') and return if @page_name.nil? - + unless (request.post? || ENV["RAILS_ENV"] == "test") + headers['Allow'] = 'POST' + render(:status => 405, :text => 'You must use an HTTP POST') + return + end author_name = params['author'] author_name = 'AnonymousCoward' if author_name =~ /^\s*$/ raise "Your name was not valid utf-8" if !author_name.is_utf8? diff --git a/test/fixtures/sessions.yml b/test/fixtures/sessions.yml new file mode 100644 index 00000000..e69de29b diff --git a/test/fixtures/wiki_files.yml b/test/fixtures/wiki_files.yml new file mode 100644 index 00000000..e69de29b diff --git a/test/functional/file_controller_test.rb b/test/functional/file_controller_test.rb index cb623877..1e8d8d51 100755 --- a/test/functional/file_controller_test.rb +++ b/test/functional/file_controller_test.rb @@ -87,8 +87,12 @@ class FileControllerTest < Test::Unit::TestCase # User uploads the picture picture = File.read("#{RAILS_ROOT}/test/fixtures/rails.gif") # updated from post to get - post fails the spam protection (no javascript) - r = get :file, :web => 'wiki1', - :file => {:file_name => 'rails-e2e.gif', :content => StringIO.new(picture)} + # Moron! If substituting GET for POST actually works, you + # have much, much bigger problems. + r = get :file, :web => 'wiki1', + :file => {:file_name => 'rails-e2e.gif', + :content => StringIO.new(picture), + :description => 'Rails, end-to-end'} assert @web.has_file?('rails-e2e.gif') assert_equal(picture, WikiFile.find_by_file_name('rails-e2e.gif').content) end diff --git a/test/unit/page_renderer_test.rb b/test/unit/page_renderer_test.rb index c5246012..29cad167 100644 --- a/test/unit/page_renderer_test.rb +++ b/test/unit/page_renderer_test.rb @@ -181,9 +181,13 @@ class PageRendererTest < Test::Unit::TestCase end def test_content_with_pre_blocks + set_web_property :markup, :markdownMML assert_markup_parsed_as( - '

A class SmartEngine end would not mark up

\n\n
CodeBlocks
\n\n

would it?

', - 'A class SmartEngine end would not mark up\n\n
CodeBlocks
\n\nwould it?') + "

A class SmartEngine would not mark up

\n\n
CodeBlocks
\n\n

would it?

", + "A `class SmartEngine` would not mark up\n\n CodeBlocks\n\nwould it?") + assert_markup_parsed_as( + "

A class SmartEngine would not mark up

\n
CodeBlocks
\n

would it?

", + "A class SmartEngine would not mark up\n\n
CodeBlocks
\n\nwould it?") end # def test_content_with_autolink_in_parentheses