Files upload restriction by files extensions (#20008).

git-svn-id: http://svn.redmine.org/redmine/trunk@14792 e93f8b46-1217-0410-a6f0-8f06a7374b81

6 files changed, 81 insertions, 1 deletions

diff --git a/app/models/attachment.rb b/app/models/attachment.rb
index 5fa89ee5a..0064555b9 100644
--- a/app/models/attachment.rb
+++ b/app/models/attachment.rb
@@ -26,7 +26,7 @@ class Attachment < ActiveRecord::Base
   validates_length_of :filename, :maximum => 255
   validates_length_of :disk_filename, :maximum => 255
   validates_length_of :description, :maximum => 255
-  validate :validate_max_file_size
+  validate :validate_max_file_size, :validate_file_extension
   attr_protected :id
 
   acts_as_event :title => :filename,
@@ -69,6 +69,15 @@ class Attachment < ActiveRecord::Base
     end
   end
 
+  def validate_file_extension
+    if @temp_file
+      extension = File.extname(filename)
+      unless self.class.valid_extension?(extension)
+        errors.add(:base, l(:error_attachment_extension_not_allowed, :extension => extension))
+      end
+    end
+  end
+
   def file=(incoming_file)
     unless incoming_file.nil?
       @temp_file = incoming_file
@@ -333,6 +342,22 @@ class Attachment < ActiveRecord::Base
     end
   end
 
+  # Returns true if the extension is allowed, otherwise false
+  def self.valid_extension?(extension)
+    extension = extension.downcase.sub(/\A\.+/, '')
+
+    denied, allowed = [:attachment_extensions_denied, :attachment_extensions_allowed].map do |setting|
+      Setting.send(setting).to_s.split(",").map {|s| s.strip.downcase.sub(/\A\.+/, '')}.reject(&:blank?)
+    end
+    if denied.present? && denied.include?(extension)
+      return false
+    end
+    unless allowed.blank? || allowed.include?(extension)
+      return false
+    end
+    true
+  end
+
   private
 
   # Physically deletes the file from the file system
diff --git a/app/views/settings/_attachments.html.erb b/app/views/settings/_attachments.html.erb
index f8a85c748..818845e55 100644
--- a/app/views/settings/_attachments.html.erb
+++ b/app/views/settings/_attachments.html.erb
@@ -3,6 +3,12 @@
 <div class="box tabular settings">
 <p><%= setting_text_field :attachment_max_size, :size => 6 %> <%= l(:"number.human.storage_units.units.kb") %></p>
 
+<p><%= setting_text_area :attachment_extensions_allowed %>
+<em class="info"><%= l(:text_comma_separated) %> <%= l(:label_example) %>: txt, png</em></p>
+
+<p><%= setting_text_area :attachment_extensions_denied %>
+<em class="info"><%= l(:text_comma_separated) %> <%= l(:label_example) %>: js, swf</em></p>
+
 <p><%= setting_text_field :file_max_size_displayed, :size => 6 %> <%= l(:"number.human.storage_units.units.kb") %></p>
 
 <p><%= setting_text_field :diff_max_lines_displayed, :size => 6 %></p>
diff --git a/config/locales/en.yml b/config/locales/en.yml
index 5158d4c0a..4ea8ecda3 100644
--- a/config/locales/en.yml
+++ b/config/locales/en.yml
@@ -210,6 +210,7 @@ en:
   error_invalid_file_encoding: "The file is not a valid %{encoding} encoded file"
   error_invalid_csv_file_or_settings: "The file is not a CSV file or does not match the settings below"
   error_can_not_read_import_file: "An error occurred while reading the file to import"
+  error_attachment_extension_not_allowed: "Attachment extension %{extension} is not allowed"
 
   mail_subject_lost_password: "Your %{value} password"
   mail_body_lost_password: 'To change your password, click on the following link:'
@@ -426,6 +427,8 @@ en:
   setting_link_copied_issue: Link issues on copy
   setting_max_additional_emails: Maximum number of additional email addresses
   setting_search_results_per_page: Search results per page
+  setting_attachment_extensions_allowed: Allowed extensions
+  setting_attachment_extensions_denied: Disallowed extensions
 
   permission_add_project: Create project
   permission_add_subprojects: Create subprojects
diff --git a/config/locales/fr.yml b/config/locales/fr.yml
index e5fbd17c0..e2bc59b45 100644
--- a/config/locales/fr.yml
+++ b/config/locales/fr.yml
@@ -230,6 +230,7 @@ fr:
   error_invalid_file_encoding: "Le fichier n'est pas un fichier %{encoding} valide"
   error_invalid_csv_file_or_settings: "Le fichier n'est pas un fichier CSV ou n'est pas conforme aux paramètres sélectionnés"
   error_can_not_read_import_file: "Une erreur est survenue lors de la lecture du fichier à importer"
+  error_attachment_extension_not_allowed: "L'extension %{extension} n'est pas autorisée"
 
   mail_subject_lost_password: "Votre mot de passe %{value}"
   mail_body_lost_password: 'Pour changer votre mot de passe, cliquez sur le lien suivant :'
@@ -446,6 +447,8 @@ fr:
   setting_link_copied_issue: Lier les demandes lors de la copie
   setting_max_additional_emails: Nombre maximal d'adresses email additionnelles
   setting_search_results_per_page: Résultats de recherche affichés par page
+  setting_attachment_extensions_allowed: Extensions autorisées
+  setting_attachment_extensions_denied: Extensions non autorisées
 
   permission_add_project: Créer un projet
   permission_add_subprojects: Créer des sous-projets
diff --git a/config/settings.yml b/config/settings.yml
index fa5e0be88..7180dac3f 100644
--- a/config/settings.yml
+++ b/config/settings.yml
@@ -55,6 +55,10 @@ session_timeout:
 attachment_max_size:
   format: int
   default: 5120
+attachment_extensions_allowed:
+  default:
+attachment_extensions_denied:
+  default:
 issues_export_limit:
   format: int
   default: 500
diff --git a/test/unit/attachment_test.rb b/test/unit/attachment_test.rb
index eaaa1459f..db274dbb9 100644
--- a/test/unit/attachment_test.rb
+++ b/test/unit/attachment_test.rb
@@ -122,6 +122,45 @@ class AttachmentTest < ActiveSupport::TestCase
     end
   end
 
+  def test_extension_should_be_validated_against_allowed_extensions
+    with_settings :attachment_extensions_allowed => "txt, png" do
+      a = Attachment.new(:container => Issue.find(1),
+                         :file => mock_file_with_options(:original_filename => "test.png"),
+                         :author => User.find(1))
+      assert_save a
+
+      a = Attachment.new(:container => Issue.find(1),
+                         :file => mock_file_with_options(:original_filename => "test.jpeg"),
+                         :author => User.find(1))
+      assert !a.save
+    end
+  end
+
+  def test_extension_should_be_validated_against_denied_extensions
+    with_settings :attachment_extensions_denied => "txt, png" do
+      a = Attachment.new(:container => Issue.find(1),
+                         :file => mock_file_with_options(:original_filename => "test.jpeg"),
+                         :author => User.find(1))
+      assert_save a
+
+      a = Attachment.new(:container => Issue.find(1),
+                         :file => mock_file_with_options(:original_filename => "test.png"),
+                         :author => User.find(1))
+      assert !a.save
+    end
+  end
+
+  def test_valid_extension_should_be_case_insensitive
+    with_settings :attachment_extensions_allowed => "txt, Png" do
+      assert Attachment.valid_extension?(".pnG")
+      assert !Attachment.valid_extension?(".jpeg")
+    end
+    with_settings :attachment_extensions_denied => "txt, Png" do
+      assert !Attachment.valid_extension?(".pnG")
+      assert Attachment.valid_extension?(".jpeg")
+    end
+  end
+
   def test_description_length_should_be_validated
     a = Attachment.new(:description => 'a' * 300)
     assert !a.save