Add latest changes from gitlab-org/gitlab@master

3 files changed, 31 insertions, 2 deletions

diff --git a/app/uploaders/gitlab_uploader.rb b/app/uploaders/gitlab_uploader.rb
index 654bb15378c..411d8b2614f 100644
--- a/app/uploaders/gitlab_uploader.rb
+++ b/app/uploaders/gitlab_uploader.rb
@@ -5,6 +5,10 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   class_attribute :options
 
+  PROTECTED_METHODS = %i(filename cache_dir work_dir store_dir).freeze
+
+  ObjectNotReadyError = Class.new(StandardError)
+
   class << self
     # DSL setter
     def storage_options(options)
@@ -33,6 +37,8 @@ class GitlabUploader < CarrierWave::Uploader::Base
 
   delegate :base_dir, :file_storage?, to: :class
 
+  before :cache, :protect_from_path_traversal!
+
   def initialize(model, mounted_as = nil, **uploader_context)
     super(model, mounted_as)
   end
@@ -121,6 +127,9 @@ class GitlabUploader < CarrierWave::Uploader::Base
   # For example, `FileUploader` builds the storage path based on the associated
   # project model's `path_with_namespace` value, which can change when the
   # project or its containing namespace is moved or renamed.
+  #
+  # When implementing this method, raise `ObjectNotReadyError` if the model
+  # does not yet exist, as it will be tested in `#protect_from_path_traversal!`
   def dynamic_segment
     raise(NotImplementedError)
   end
@@ -138,4 +147,21 @@ class GitlabUploader < CarrierWave::Uploader::Base
   def pathname
     @pathname ||= Pathname.new(path)
   end
+
+  # Protect against path traversal attacks
+  # This takes a list of methods to test for path traversal, e.g. ../../
+  # and checks each of them. This uses `.send` so that any potential errors
+  # don't block the entire set from being tested.
+  #
+  # @param [CarrierWave::SanitizedFile]
+  # @return [Nil]
+  # @raise [Gitlab::Utils::PathTraversalAttackError]
+  def protect_from_path_traversal!(file)
+    PROTECTED_METHODS.each do |method|
+      Gitlab::Utils.check_path_traversal!(self.send(method)) # rubocop: disable GitlabSecurity/PublicSend
+
+    rescue ObjectNotReadyError
+      # Do nothing. This test was attempted before the file was ready for that method
+    end
+  end
 end
diff --git a/app/uploaders/job_artifact_uploader.rb b/app/uploaders/job_artifact_uploader.rb
index 47976c909e8..83dc1030606 100644
--- a/app/uploaders/job_artifact_uploader.rb
+++ b/app/uploaders/job_artifact_uploader.rb
@@ -4,7 +4,6 @@ class JobArtifactUploader < GitlabUploader
   extend Workhorse::UploadPath
   include ObjectStorage::Concern
 
-  ObjectNotReadyError = Class.new(StandardError)
   UnknownFileLocationError = Class.new(StandardError)
 
   storage_options Gitlab.config.artifacts
@@ -24,7 +23,9 @@ class JobArtifactUploader < GitlabUploader
   private
 
   def dynamic_segment
-    raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id
+    # This now tests model.created_at because it can for some reason be nil in the test suite,
+    # and it's not clear if this is intentional or not
+    raise ObjectNotReadyError, 'JobArtifact is not ready' unless model.id && model.created_at
 
     if model.hashed_path?
       hashed_path
diff --git a/app/uploaders/packages/package_file_uploader.rb b/app/uploaders/packages/package_file_uploader.rb
index 28545b9fcdf..61fbe2b4c49 100644
--- a/app/uploaders/packages/package_file_uploader.rb
+++ b/app/uploaders/packages/package_file_uploader.rb
@@ -20,6 +20,8 @@ class Packages::PackageFileUploader < GitlabUploader
   private
 
   def dynamic_segment
+    raise ObjectNotReadyError, "Package model not ready" unless model.id
+
     Gitlab::HashedPath.new('packages', model.package.id, 'files', model.id, root_hash: model.package.project_id)
   end
 end