Harden multipart upload URL and thread validation

Author: vipulnsward

lib/uploadcare/api/upload.rb

@@ -2,7 +2,9 @@
 
 require 'faraday'
 require 'faraday/multipart'
+require 'ipaddr'
 require 'mime/types'
+require 'resolv'
 require 'securerandom'
 require 'uri'
 require 'addressable/uri'
@@ -91,9 +93,9 @@ def post(path:, params: {}, headers: {}, request_options: {})
   # @return [Boolean] true on success
   # @raise [Uploadcare::Exception::MultipartUploadError] on failure after retries
   def upload_part_to_url(presigned_url, part_data, max_retries: 3)
+    uri = validated_presigned_uri(presigned_url)
     retries = 0
     begin
-      uri = URI.parse(presigned_url)
       conn = Faraday.new(url: "#{uri.scheme}://#{uri.host}") do |f|
         f.adapter Faraday.default_adapter
       end
@@ -214,4 +216,46 @@ def handle_faraday_error(error)
 
     raise Uploadcare::Exception::RequestError, "Network error: #{error.message}"
   end
+
+  def validated_presigned_uri(url)
+    uri = URI.parse(url.to_s)
+    raise ArgumentError, 'presigned_url must use HTTPS' unless uri.is_a?(URI::HTTPS)
+    raise ArgumentError, 'presigned_url host is required' if uri.host.to_s.empty?
+    raise ArgumentError, 'presigned_url cannot target localhost' if local_hostname?(uri.host)
+    raise ArgumentError, 'presigned_url cannot target a private address' if private_host?(uri.host)
+
+    uri
+  rescue URI::InvalidURIError => e
+    raise ArgumentError, "Invalid presigned_url: #{e.message}"
+  end
+
+  def local_hostname?(host)
+    normalized_host = host.to_s.downcase
+    normalized_host == 'localhost' || normalized_host.end_with?('.localhost', '.local')
+  end
+
+  def private_host?(host)
+    return private_ip?(host) if ip_literal?(host)
+
+    Resolv.getaddresses(host).any? { |address| private_ip?(address) }
+  rescue Resolv::ResolvError, SocketError
+    false
+  end
+
+  def ip_literal?(host)
+    IPAddr.new(host)
+    true
+  rescue IPAddr::InvalidAddressError
+    false
+  end
+
+  def private_ip?(address)
+    ip = IPAddr.new(address)
+    return true if ip.loopback?
+    return true if ip.link_local?
+
+    ip.private?
+  rescue IPAddr::InvalidAddressError
+    false
+  end
 end

lib/uploadcare/operations/multipart_upload.rb

@@ -77,10 +77,13 @@ def upload(file:, request_options: {}, **options, &block)
 
   def normalize_upload_options(options)
     part_size = Integer(options.fetch(:part_size, config.multipart_chunk_size || CHUNK_SIZE))
-    threads = Integer(options.fetch(:threads, 1))
+    max_threads = Integer(config.upload_threads || 1)
+    threads = Integer(options.fetch(:threads, max_threads))
 
     raise ArgumentError, 'part_size must be > 0' if part_size <= 0
+    raise ArgumentError, 'upload_threads must be >= 1' if max_threads < 1
     raise ArgumentError, 'threads must be >= 1' if threads < 1
+    raise ArgumentError, "threads must be <= #{max_threads}" if threads > max_threads
 
     [part_size, threads]
   end

spec/uploadcare/api/upload_spec.rb

@@ -136,6 +136,24 @@
       expect(result).to be true
     end
 
+    it 'rejects non-https presigned URLs' do
+      expect do
+        upload.upload_part_to_url('http://s3.amazonaws.com/bucket/part', 'data')
+      end.to raise_error(ArgumentError, 'presigned_url must use HTTPS')
+    end
+
+    it 'rejects localhost presigned URLs' do
+      expect do
+        upload.upload_part_to_url('https://localhost/bucket/part', 'data')
+      end.to raise_error(ArgumentError, 'presigned_url cannot target localhost')
+    end
+
+    it 'rejects private IP presigned URLs' do
+      expect do
+        upload.upload_part_to_url('https://10.0.0.5/bucket/part', 'data')
+      end.to raise_error(ArgumentError, 'presigned_url cannot target a private address')
+    end
+
     it 'raises MultipartUploadError after max retries on non-2xx responses' do
       stub_request(:put, presigned_url)
         .to_return(status: 500, body: 'Internal Server Error', headers: {}) # force Zeitwerk to load upload_error.rb

spec/uploadcare/operations/multipart_upload_spec.rb

@@ -106,6 +106,14 @@
         expect(result.error).to be_a(ArgumentError)
         expect(result.error.message).to eq('part_size must be > 0')
       end
+
+      it 'returns a failure Result when threads exceeds config.upload_threads' do
+        result = uploader.upload(file: tempfile, threads: 3)
+
+        expect(result.failure?).to be(true)
+        expect(result.error).to be_a(ArgumentError)
+        expect(result.error.message).to eq('threads must be <= 2')
+      end
     end
 
     context 'when performing sequential upload (threads <= 1)' do
@@ -271,13 +279,21 @@
       end
 
       it 'uploads correct data chunks in parallel' do
+        high_thread_config = Uploadcare::Configuration.new(
+          public_key: 'demopublickey',
+          secret_key: 'demosecretkey',
+          auth_type: 'Uploadcare.Simple',
+          multipart_chunk_size: 1024,
+          upload_threads: 3
+        )
+        high_thread_uploader = described_class.new(upload_client: upload_client, config: high_thread_config)
         chunks = Mutex.new
         all_chunks = {}
         allow(upload_client).to receive(:upload_part_to_url) do |url, data|
           chunks.synchronize { all_chunks[url] = data.bytesize }
         end
 
-        uploader.upload(file: tempfile, threads: 3)
+        high_thread_uploader.upload(file: tempfile, threads: 3)
         expect(all_chunks.values).to all(eq(1024))
         expect(all_chunks.size).to eq(3)
       end
@@ -300,8 +316,17 @@
         expect(final_uploaded).to eq(3072)
       end
 
-      it 'works with more threads than parts' do
-        result = uploader.upload(file: tempfile, threads: 10)
+      it 'works with more threads than parts when allowed by config' do
+        high_thread_config = Uploadcare::Configuration.new(
+          public_key: 'demopublickey',
+          secret_key: 'demosecretkey',
+          auth_type: 'Uploadcare.Simple',
+          multipart_chunk_size: 1024,
+          upload_threads: 10
+        )
+        high_thread_uploader = described_class.new(upload_client: upload_client, config: high_thread_config)
+
+        result = high_thread_uploader.upload(file: tempfile, threads: 10)
         expect(result.success?).to be(true)
       end
 
@@ -329,11 +354,19 @@
       end
 
       it 'propagates the first error from parallel workers' do
+        high_thread_config = Uploadcare::Configuration.new(
+          public_key: 'demopublickey',
+          secret_key: 'demosecretkey',
+          auth_type: 'Uploadcare.Simple',
+          multipart_chunk_size: 1024,
+          upload_threads: 3
+        )
+        high_thread_uploader = described_class.new(upload_client: upload_client, config: high_thread_config)
         allow(upload_client).to receive(:upload_part_to_url) do
           raise 'network timeout'
         end
 
-        result = uploader.upload(file: tempfile, threads: 3)
+        result = high_thread_uploader.upload(file: tempfile, threads: 3)
         expect(result.failure?).to be(true)
         expect(result.error).to be_a(RuntimeError)
       end
@@ -494,6 +527,14 @@ def seek(_pos) = nil
 
     context 'when performing parallel upload with offset >= total_size' do
       it 'handles more presigned URLs than needed' do
+        high_thread_config = Uploadcare::Configuration.new(
+          public_key: 'demopublickey',
+          secret_key: 'demosecretkey',
+          auth_type: 'Uploadcare.Simple',
+          multipart_chunk_size: 1024,
+          upload_threads: 3
+        )
+        high_thread_uploader = described_class.new(upload_client: upload_client, config: high_thread_config)
         small_content = 'C' * 1024
         small_file = Tempfile.new(['small_parallel', '.bin'])
         small_file.binmode
@@ -511,7 +552,7 @@ def seek(_pos) = nil
           uploaded_urls << url
         end
 
-        result = uploader.upload(file: small_file, threads: 3)
+        result = high_thread_uploader.upload(file: small_file, threads: 3)
         expect(result.success?).to be(true)
         expect(uploaded_urls.length).to eq(1)