From 5d15e3f39d13cb9c301edc7514f47e41f1c61867 Mon Sep 17 00:00:00 2001 From: Jacques Distler Date: Mon, 26 Jan 2009 00:21:30 -0600 Subject: [PATCH] Security: Instiki 0.16.2 On Webs with file uploads enabled, uploaded files were stored (in version 0.16.1 and earlier) in the public/ directory. This was a security threat. A miscreant could upload a .html file. When a user clicked on the link to the file, it was opened (unsanitized) in the browser. As of version 0.16.2, uploaded files are stored in the webs/ directory. Now, when the user clicks on the link, the file is sent with the Content-Disposition: attachment header set, which causes the file to be downloaded, rather than opened in the browser. As always, files downloaded from the internets should be treated with caution. At least, this way, they are not aoutomatically opened in the browser. To move your existing uploaded files to the new location, do a rake upgrade_instiki --- app/controllers/application.rb | 4 ++-- app/models/web.rb | 4 ++-- lib/tasks/upgrade_instiki.rake | 15 +++++++++++++++ test/functional/file_controller_test.rb | 2 +- test/unit/page_renderer_test.rb | 4 ++-- test/unit/wiki_file_test.rb | 4 ++-- 6 files changed, 24 insertions(+), 9 deletions(-) create mode 100644 lib/tasks/upgrade_instiki.rake diff --git a/app/controllers/application.rb b/app/controllers/application.rb index 95b6a2b4..21e7d7d7 100644 --- a/app/controllers/application.rb +++ b/app/controllers/application.rb @@ -241,9 +241,9 @@ module Instiki module VERSION #:nodoc: MAJOR = 0 MINOR = 16 - TINY = 1 + TINY = 2 SUFFIX = '(MML+)' - PRERELEASE = 'pre' # false + PRERELEASE = false if PRERELEASE STRING = [MAJOR, MINOR].join('.') + PRERELEASE + SUFFIX else diff --git a/app/models/web.rb b/app/models/web.rb index 8adc0d70..50d15347 100644 --- a/app/models/web.rb +++ b/app/models/web.rb @@ -153,9 +153,9 @@ class Web < ActiveRecord::Base def files_path if default_web? - "#{RAILS_ROOT}/public/files" + "#{RAILS_ROOT}/webs/files" else - "#{RAILS_ROOT}/public/#{self.address}/files" + "#{RAILS_ROOT}/webs/#{self.address}/files" end end end diff --git a/lib/tasks/upgrade_instiki.rake b/lib/tasks/upgrade_instiki.rake new file mode 100644 index 00000000..49162073 --- /dev/null +++ b/lib/tasks/upgrade_instiki.rake @@ -0,0 +1,15 @@ +require 'sqlite3' + +task :upgrade_instiki do + db = SQLite3::Database.new( "db/production.db.sqlite3" ) + db.execute( "select * from webs" ) do |row| + if File.exists?('public/' + row[4]) + if File.exists?('webs/' + row[4]) + print "Warning! The directory webs/#{row[4]} already exists. Skipping.\n" + else + File.rename('public/' + row[4], 'webs/' + row[4]) + print "Moved: #{row[4]}\n" + end + end + end +end diff --git a/test/functional/file_controller_test.rb b/test/functional/file_controller_test.rb index 07658ca4..610794b6 100755 --- a/test/functional/file_controller_test.rb +++ b/test/functional/file_controller_test.rb @@ -24,7 +24,7 @@ class FileControllerTest < Test::Unit::TestCase @wiki = Wiki.new WikiFile.delete_all require 'fileutils' - FileUtils.rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") + FileUtils.rm_rf("#{RAILS_ROOT}/webs/wiki1/files/*") end def test_file_upload_form diff --git a/test/unit/page_renderer_test.rb b/test/unit/page_renderer_test.rb index fae92830..fd29b826 100644 --- a/test/unit/page_renderer_test.rb +++ b/test/unit/page_renderer_test.rb @@ -510,7 +510,7 @@ END_THM def test_link_to_pic_and_file WikiFile.delete_all require 'fileutils' - FileUtils.rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") + FileUtils.rm_rf("#{RAILS_ROOT}/webs/wiki1/files/*") @web.wiki_files.create(:file_name => 'square.jpg', :description => 'Square', :content => 'never mind') assert_markup_parsed_as( "

Blue Square

", @@ -529,7 +529,7 @@ END_THM def test_link_to_pic_and_file_null_desc WikiFile.delete_all require 'fileutils' - FileUtils.rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") + FileUtils.rm_rf("#{RAILS_ROOT}/webs/wiki1/files/*") @web.wiki_files.create(:file_name => 'square.jpg', :description => '', :content => 'never mind') assert_markup_parsed_as( "

Blue Square

", diff --git a/test/unit/wiki_file_test.rb b/test/unit/wiki_file_test.rb index 233a73fa..886bd36d 100644 --- a/test/unit/wiki_file_test.rb +++ b/test/unit/wiki_file_test.rb @@ -7,8 +7,8 @@ class WikiFileTest < Test::Unit::TestCase def setup @web = webs(:test_wiki) - mkdir_p("#{RAILS_ROOT}/public/wiki1/files/") - rm_rf("#{RAILS_ROOT}/public/wiki1/files/*") + mkdir_p("#{RAILS_ROOT}/webs/wiki1/files/") + rm_rf("#{RAILS_ROOT}/webs/wiki1/files/*") WikiFile.delete_all end