Make UploadedFile to accept only Remotedisallow-local-storage

2 files changed, 36 insertions, 31 deletions

diff --git a/app/uploaders/object_storage.rb b/app/uploaders/object_storage.rb
index e395e52f5c2..88a63b6e5ce 100644
--- a/app/uploaders/object_storage.rb
+++ b/app/uploaders/object_storage.rb
@@ -319,7 +319,7 @@ module ObjectStorage
     def cache!(new_file = sanitized_file)
       # We intercept ::UploadedFile which might be stored on remote storage
       # We use that for "accelerated" uploads, where we store result on remote storage
-      if new_file.is_a?(::UploadedFile) && new_file.remote_id
+      if new_file.is_a?(::UploadedFile) && new_file.remote?
         return cache_remote_file!(new_file.remote_id, new_file.original_filename)
       end
 
diff --git a/lib/uploaded_file.rb b/lib/uploaded_file.rb
index 5dc85b2baea..0aa453612e8 100644
--- a/lib/uploaded_file.rb
+++ b/lib/uploaded_file.rb
@@ -8,63 +8,68 @@ class UploadedFile
   # The filename, *not* including the path, of the "uploaded" file
   attr_reader :original_filename
 
-  # The tempfile
-  attr_reader :tempfile
-
   # The content type of the "uploaded" file
   attr_accessor :content_type
 
+  attr_reader :local_path
   attr_reader :remote_id
   attr_reader :sha256
 
-  def initialize(path, filename: nil, content_type: "application/octet-stream", sha256: nil, remote_id: nil)
-    raise InvalidPathError, "#{path} file does not exist" unless ::File.exist?(path)
+  def initialize(local_path, filename: nil, content_type: "application/octet-stream", sha256: nil, remote_id: nil, allowed_paths: nil)
+    raise InvalidPathError, "missing filename or local_path" unless filename || local_path
+    raise InvalidPathError, "local_path or remote_id has to be provided" unless local_path || remote_id
 
-    @content_type = content_type
-    @original_filename = filename || ::File.basename(path)
+    if local_path
+      raise InvalidPathError, "#{path} file does not exist" unless ::File.exist?(local_path)
+      raise InvalidPathError, "insecure local_path used for '#{local_path}'" unless self.class.allowed_path?(local_path, allowed_paths)
+    end
+
+    @local_path = local_path
     @content_type = content_type
     @sha256 = sha256
     @remote_id = remote_id
-    @tempfile = File.new(path, 'rb')
+    @original_filename = filename
+    @original_filename ||= ::File.basename(local_path) if local_path
   end
 
   def self.from_params(params, field, upload_path)
-    unless params["#{field}.path"]
-      raise InvalidPathError, "file is invalid" if params["#{field}.remote_id"]
-
-      return
-    end
-
-    file_path = File.realpath(params["#{field}.path"])
-
-    unless self.allowed_path?(file_path, [upload_path, Dir.tmpdir].compact)
-      raise InvalidPathError, "insecure path used '#{file_path}'"
-    end
-
-    UploadedFile.new(file_path,
+    UploadedFile.new(params["#{field}.path"],
       filename: params["#{field}.name"],
       content_type: params["#{field}.type"] || 'application/octet-stream',
       sha256: params["#{field}.sha256"],
-      remote_id: params["#{field}.remote_id"])
+      remote_id: params["#{field}.remote_id"],
+      allowed_paths: [upload_path, Dir.tmpdir].compact)
   end
 
-  def self.allowed_path?(file_path, paths)
-    paths.any? do |path|
-      File.exist?(path) && file_path.start_with?(File.realpath(path))
+  def self.allowed_path?(file_path, allowed_paths)
+    return true unless allowed_paths
+
+    file_path = File.realpath(file_path)
+
+    allowed_paths.any? do |allowed_path|
+      File.exist?(allowed_path) && file_path.start_with?(File.realpath(allowed_path))
     end
   end
 
-  def path
-    @tempfile.path
+  def local?
+    local_path.present?
   end
 
-  alias_method :local_path, :path
+  def remote?
+    remote_id.present?
+  end
+
+  alias_method :path, :local_path
+
+  def tempfile
+    @tempfile ||= File.new(local_path, 'rb') if local_path
+  end
 
   def method_missing(method_name, *args, &block) #:nodoc:
-    @tempfile.__send__(method_name, *args, &block) # rubocop:disable GitlabSecurity/PublicSend
+    tempfile.__send__(method_name, *args, &block) # rubocop:disable GitlabSecurity/PublicSend
   end
 
   def respond_to?(method_name, include_private = false) #:nodoc:
-    @tempfile.respond_to?(method_name, include_private) || super
+    tempfile&.respond_to?(method_name, include_private) || super
   end
 end