Download as
Browse Code »

Commit 393b63329b67aba61f420082f06eb7ec9c4ffe73

Authored by Rodrigo Souto
1 parent 635a6c6c
Exists in master and in 22 other branches angular_poc, api-article-archived, article-readonly, backup-7-jan-2016, captcha_serpro_plugin, content-manager-hostspot, export-comment-paragraph, fix_event_date_issue, login-captcha, master_rails3, new_video_plugin, pg_search_rank, production, refactor_with_role, refactor_with_role_scope, remove_profile_cat_icons, resend_confirmation_email, staging, staging_rails3, tasks_keep_filter_params, theme-brasil-digital-from-staging, travis

Avoiding malicious uploaded file 

Adding '.txt' to the end of runnable scripts to avoid attacks due to
unsecure servers. Same solution adopted by foswiki.

(ActionItem2009)
Showing 8 changed files with 44 additions and 0 deletions   Show diff stats
app/models/environment.rb
  Diff comments   View file @393b633
@@ -9,6 +9,13 @@ class Environment < ActiveRecord::Base @@ -9,6 +9,13 @@ class Environment < ActiveRecord::Base
9 9
10 has_many :tasks, :dependent => :destroy, :as => 'target' 10 has_many :tasks, :dependent => :destroy, :as => 'target'
11 11
  12 + IDENTIFY_SCRIPTS = /(?:php[0-9s]?(\..*)?|[sp]htm[l]?(\..*)?|pl|py|cgi|rb)/
  13 +
  14 + def self.verify_filename(filename)
  15 + filename += '.txt' if filename =~ IDENTIFY_SCRIPTS
  16 + return filename
  17 + end
  18 +
12 PERMISSIONS['Environment'] = { 19 PERMISSIONS['Environment'] = {
13 'view_environment_admin_panel' => N_('View environment admin panel'), 20 'view_environment_admin_panel' => N_('View environment admin panel'),
14 'edit_environment_features' => N_('Edit environment features'), 21 'edit_environment_features' => N_('Edit environment features'),
app/models/image.rb
  Diff comments   View file @393b633
@@ -4,6 +4,8 @@ class Image < ActiveRecord::Base @@ -4,6 +4,8 @@ class Image < ActiveRecord::Base
4 Image.attachment_options[:max_size] 4 Image.attachment_options[:max_size]
5 end 5 end
6 6
  7 + before_create { |file| file.filename = Environment.verify_filename(file.filename) }
  8 +
7 has_attachment :content_type => :image, 9 has_attachment :content_type => :image,
8 :storage => :file_system, 10 :storage => :file_system,
9 :path_prefix => 'public/image_uploads', 11 :path_prefix => 'public/image_uploads',
app/models/thumbnail.rb
  Diff comments   View file @393b633
@@ -3,5 +3,7 @@ class Thumbnail < ActiveRecord::Base @@ -3,5 +3,7 @@ class Thumbnail < ActiveRecord::Base
3 :content_type => :image, :max_size => 5.megabytes 3 :content_type => :image, :max_size => 5.megabytes
4 validates_as_attachment 4 validates_as_attachment
5 5
  6 + before_create { |file| file.filename = Environment.verify_filename(file.filename) }
  7 +
6 postgresql_attachment_fu 8 postgresql_attachment_fu
7 end 9 end
app/models/uploaded_file.rb
  Diff comments   View file @393b633
@@ -20,6 +20,7 @@ class UploadedFile < Article @@ -20,6 +20,7 @@ class UploadedFile < Article
20 20
21 before_create do |uploaded_file| 21 before_create do |uploaded_file|
22 uploaded_file.is_image = true if uploaded_file.image? 22 uploaded_file.is_image = true if uploaded_file.image?
  23 + uploaded_file.filename = Environment.verify_filename(uploaded_file.filename)
23 end 24 end
24 25
25 def thumbnail_path 26 def thumbnail_path
test/unit/environment_test.rb
  Diff comments   View file @393b633
@@ -1125,4 +1125,21 @@ class EnvironmentTest < Test::Unit::TestCase @@ -1125,4 +1125,21 @@ class EnvironmentTest < Test::Unit::TestCase
1125 assert_equal 'localhost:9999', env.default_hostname 1125 assert_equal 'localhost:9999', env.default_hostname
1126 end 1126 end
1127 1127
  1128 + should 'identify scripts with regex' do
  1129 + scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
  1130 + name = 'uploaded_file'
  1131 + scripts_extensions.each do |extension|
  1132 + assert_not_nil name+'.'+extension =~ Environment::IDENTIFY_SCRIPTS
  1133 + end
  1134 + end
  1135 +
  1136 + should 'verify filename and append .txt if script' do
  1137 + scripts_extensions = %w[php php1 php4 phps php.bli cgi shtm phtm shtml phtml pl py rb]
  1138 + name = 'uploaded_file'
  1139 + scripts_extensions.each do |extension|
  1140 + filename = name+'.'+extension
  1141 + assert_equal filename+'.txt', Environment.verify_filename(filename)
  1142 + end
  1143 + end
  1144 +
1128 end 1145 end
test/unit/image_test.rb
  Diff comments   View file @393b633
@@ -118,4 +118,9 @@ class ImageTest < Test::Unit::TestCase @@ -118,4 +118,9 @@ class ImageTest < Test::Unit::TestCase
118 file.destroy 118 file.destroy
119 end 119 end
120 120
  121 + should 'not allow script files to be uploaded without append .txt in the end' do
  122 + file = Image.create!(:uploaded_data => fixture_file_upload('files/hello_world.php', 'image/png'))
  123 + assert_equal 'hello_world.php.txt', file.filename
  124 + end
  125 +
121 end 126 end
test/unit/thumbnail_test.rb
  Diff comments   View file @393b633
@@ -9,5 +9,10 @@ class ThumbnailTest < Test::Unit::TestCase @@ -9,5 +9,10 @@ class ThumbnailTest < Test::Unit::TestCase
9 assert_match 'image/', item 9 assert_match 'image/', item
10 end 10 end
11 end 11 end
  12 +
  13 + should 'not allow script files to be uploaded without append .txt in the end' do
  14 + file = Thumbnail.create!(:uploaded_data => fixture_file_upload('files/hello_world.php', 'image/png'))
  15 + assert_equal 'hello_world.php.txt', file.filename
  16 + end
12 17
13 end 18 end
test/unit/uploaded_file_test.rb
  Diff comments   View file @393b633
@@ -325,4 +325,9 @@ class UploadedFileTest < Test::Unit::TestCase @@ -325,4 +325,9 @@ class UploadedFileTest < Test::Unit::TestCase
325 uses_sqlite 325 uses_sqlite
326 end 326 end
327 327
  328 + should 'not allow script files to be uploaded without append .txt in the end' do
  329 + file = UploadedFile.create!(:uploaded_data => fixture_file_upload('files/hello_world.php', 'application/x-php'), :profile => @profile)
  330 + assert_equal 'hello_world.php.txt', file.filename
  331 + end
  332 +
328 end 333 end