Tags: security, patch

Dear maintainers,

I took the patches from upstream and rebased them; so far everything works
fine on our systems. I'd very much apprechiate an officially patched
version in buster-backports.

CVE-2021-31863: 
https://www.redmine.org/projects/redmine/repository/revisions/20854
CVE-2021-31864: 
https://www.redmine.org/projects/redmine/repository/revisions/20946
CVE-2021-31865: 
https://www.redmine.org/projects/redmine/repository/revisions/20970
CVE-2021-31866: 
https://www.redmine.org/projects/redmine/repository/revisions/20962

best regards,
    Adi Kriegisch
Index: redmine-4.0.7/app/models/token.rb
===================================================================
--- redmine-4.0.7.orig/app/models/token.rb
+++ redmine-4.0.7/app/models/token.rb
@@ -113,11 +113,13 @@ class Token < ActiveRecord::Base
     return nil unless action.present? && key =~ /\A[a-z0-9]+\z/i
 
     token = Token.find_by(:action => action, :value => key)
-    if token && (token.action == action) && (token.value == key) && token.user
-      if validity_days.nil? || (token.created_on > validity_days.days.ago)
-        token
-      end
-    end
+    return unless token
+    return unless token.action == action
+    return unless ActiveSupport::SecurityUtils.secure_compare(token.value.to_s, key)
+    return unless token.user
+    return unless validity_days.nil? || (token.created_on > validity_days.days.ago)
+
+    token
   end
 
   def self.generate_token_value
Index: redmine-4.0.7/app/controllers/sys_controller.rb
===================================================================
--- redmine-4.0.7.orig/app/controllers/sys_controller.rb
+++ redmine-4.0.7/app/controllers/sys_controller.rb
@@ -16,6 +16,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class SysController < ActionController::Base
+  include ActiveSupport::SecurityUtils
+
   before_action :check_enabled
 
   def projects
@@ -74,7 +76,7 @@ class SysController < ActionController::
 
   def check_enabled
     User.current = nil
-    unless Setting.sys_api_enabled? && params[:key].to_s == Setting.sys_api_key
+    unless Setting.sys_api_enabled? && secure_compare(params[:key].to_s, Setting.sys_api_key.to_s)
       render :plain => 'Access denied. Repository management WS is disabled or key is invalid.', :status => 403
       return false
     end
Index: redmine-4.0.7/app/controllers/mail_handler_controller.rb
===================================================================
--- redmine-4.0.7.orig/app/controllers/mail_handler_controller.rb
+++ redmine-4.0.7/app/controllers/mail_handler_controller.rb
@@ -16,6 +16,8 @@
 # Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301, USA.
 
 class MailHandlerController < ActionController::Base
+  include ActiveSupport::SecurityUtils
+
   before_action :check_credential
 
   # Displays the email submission form
@@ -37,7 +39,7 @@ class MailHandlerController < ActionCont
 
   def check_credential
     User.current = nil
-    unless Setting.mail_handler_api_enabled? && params[:key].to_s == Setting.mail_handler_api_key
+    unless Setting.mail_handler_api_enabled? && secure_compare(params[:key].to_s, Setting.mail_handler_api_key.to_s)
       render :plain => 'Access denied. Incoming emails WS is disabled or key is invalid.', :status => 403
     end
   end
Index: redmine-4.0.7/test/unit/attachment_test.rb
===================================================================
--- redmine-4.0.7.orig/test/unit/attachment_test.rb
+++ redmine-4.0.7/test/unit/attachment_test.rb
@@ -151,6 +151,19 @@ class AttachmentTest < ActiveSupport::Te
     end
   end
 
+  def test_extension_update_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
+
+      b = Attachment.find(a.id)
+      b.filename = "test.png"
+      assert !b.save
+    end
+  end
+
   def test_valid_extension_should_be_case_insensitive
     with_settings :attachment_extensions_allowed => "txt, Png" do
       assert Attachment.valid_extension?(".pnG")
Index: redmine-4.0.7/app/models/attachment.rb
===================================================================
--- redmine-4.0.7.orig/app/models/attachment.rb
+++ redmine-4.0.7/app/models/attachment.rb
@@ -27,7 +27,8 @@ 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_file_extension
+  validate :validate_max_file_size
+  validate :validate_file_extension, :if => :filename_changed?
 
   acts_as_event :title => :filename,
                 :url => Proc.new {|o| {:controller => 'attachments', :action => 'show', :id => o.id, :filename => o.filename}}
@@ -74,11 +75,9 @@ class Attachment < ActiveRecord::Base
   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
+    extension = File.extname(filename)
+    unless self.class.valid_extension?(extension)
+      errors.add(:base, l(:error_attachment_extension_not_allowed, :extension => extension))
     end
   end
 
Index: redmine-4.0.7/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
===================================================================
--- redmine-4.0.7.orig/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
+++ redmine-4.0.7/lib/plugins/acts_as_attachable/lib/acts_as_attachable.rb
@@ -105,7 +105,7 @@ module Redmine
               end
               next unless a
               a.description = attachment['description'].to_s.strip
-              if a.new_record?
+              if a.new_record? || a.invalid?
                 unsaved_attachments << a
               else
                 saved_attachments << a
Index: redmine-4.0.7/app/models/mail_handler.rb
===================================================================
--- redmine-4.0.7.orig/app/models/mail_handler.rb
+++ redmine-4.0.7/app/models/mail_handler.rb
@@ -231,8 +231,7 @@ class MailHandler < ActionMailer::Base
     return unless issue
     # check permission
     unless handler_options[:no_permission_check]
-      unless user.allowed_to?(:add_issue_notes, issue.project) ||
-               user.allowed_to?(:edit_issues, issue.project)
+      unless issue.notes_addable?
         raise UnauthorizedAction
       end
     end
Index: redmine-4.0.7/app/models/repository.rb
===================================================================
--- redmine-4.0.7.orig/app/models/repository.rb
+++ redmine-4.0.7/app/models/repository.rb
@@ -459,6 +459,10 @@ class Repository < ActiveRecord::Base
     scope
   end
 
+  def valid_name?(name)
+    scm.valid_name?(name)
+  end
+
   protected
 
   # Validates repository url based against an optional regular expression
Index: redmine-4.0.7/app/controllers/repositories_controller.rb
===================================================================
--- redmine-4.0.7.orig/app/controllers/repositories_controller.rb
+++ redmine-4.0.7/app/controllers/repositories_controller.rb
@@ -305,7 +305,7 @@ class RepositoriesController < Applicati
     render_404
   end
 
-  REV_PARAM_RE = %r{\A[a-f0-9]*\Z}i
+  REV_PARAM_RE = %r{\A[a-f0-9]*\z}i
 
   def find_project_repository
     @project = Project.find(params[:id])
@@ -316,14 +316,12 @@ class RepositoriesController < Applicati
     end
     (render_404; return false) unless @repository
     @path = params[:path].is_a?(Array) ? params[:path].join('/') : params[:path].to_s
-    @rev = params[:rev].blank? ? @repository.default_branch : params[:rev].to_s.strip
-    @rev_to = params[:rev_to]
 
-    unless @rev.to_s.match(REV_PARAM_RE) && @rev_to.to_s.match(REV_PARAM_RE)
-      if @repository.branches.blank?
-        raise InvalidRevisionParam
-      end
-    end
+    @rev = params[:rev].to_s.strip.presence || @repository.default_branch
+    raise InvalidRevisionParam unless valid_name?(@rev)
+
+    @rev_to = params[:rev_to].to_s.strip.presence
+    raise InvalidRevisionParam unless valid_name?(@rev_to)
   rescue ActiveRecord::RecordNotFound
     render_404
   rescue InvalidRevisionParam
@@ -408,4 +406,11 @@ class RepositoriesController < Applicati
       'attachment'
     end
   end
+
+  def valid_name?(rev)
+    return true if rev.nil?
+    return true if REV_PARAM_RE.match?(rev)
+
+    @repository ? @repository.valid_name?(rev) : true
+  end
 end
Index: redmine-4.0.7/lib/redmine/scm/adapters/abstract_adapter.rb
===================================================================
--- redmine-4.0.7.orig/lib/redmine/scm/adapters/abstract_adapter.rb
+++ redmine-4.0.7/lib/redmine/scm/adapters/abstract_adapter.rb
@@ -176,6 +176,14 @@ module Redmine
           (path[-1,1] == "/") ? path[0..-2] : path
         end
 
+        def valid_name?(name)
+          return true if name.nil?
+          return true if name.is_a?(Integer) && name > 0
+          return true if name.is_a?(String) && name =~ /\A[0-9]*\z/
+
+          false
+        end
+
       private
         def retrieve_root_url
           info = self.info
Index: redmine-4.0.7/lib/redmine/scm/adapters/git_adapter.rb
===================================================================
--- redmine-4.0.7.orig/lib/redmine/scm/adapters/git_adapter.rb
+++ redmine-4.0.7/lib/redmine/scm/adapters/git_adapter.rb
@@ -380,6 +380,18 @@ module Redmine
           nil
         end
 
+        def valid_name?(name)
+          return false unless name.is_a?(String)
+
+          return false if name.start_with?('-', '/', 'refs/heads/', 'refs/remotes/')
+          return false if name == 'HEAD'
+
+          git_cmd ['show-ref', '--heads', '--tags', '--quiet', '--', name]
+          true
+        rescue ScmCommandAborted
+          false
+        end
+
         class Revision < Redmine::Scm::Adapters::Revision
           # Returns the readable identifier
           def format_identifier
Index: redmine-4.0.7/lib/redmine/scm/adapters/mercurial_adapter.rb
===================================================================
--- redmine-4.0.7.orig/lib/redmine/scm/adapters/mercurial_adapter.rb
+++ redmine-4.0.7/lib/redmine/scm/adapters/mercurial_adapter.rb
@@ -281,6 +281,15 @@ module Redmine
           Annotate.new
         end
 
+        def valid_name?(name)
+          return false unless name.nil? || name.is_a?(String)
+
+          # Mercurials names don't need to be checked further as its CLI
+          # interface is restrictive enough to reject any invalid names on its
+          # own.
+          true
+        end
+
         class Revision < Redmine::Scm::Adapters::Revision
           # Returns the readable identifier
           def format_identifier

Attachment: signature.asc
Description: PGP signature

Reply via email to