From ca50941dbae32581bca7c6afdb0903f6beeec38d Mon Sep 17 00:00:00 2001 From: "Abdallah Ebrahim (dracula)" Date: Wed, 19 Aug 2026 20:18:28 +0300 Subject: [PATCH 1/4] fix: pin SSRF-safe resolution --- app/models/upload.rb | 157 ++++++++++-------- .../api/v4/uploads_controller_test.rb | 37 ++--- 2 files changed, 99 insertions(+), 95 deletions(-) diff --git a/app/models/upload.rb b/app/models/upload.rb index 3550185..39b88ef 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "open-uri" -require "resolv" require "ipaddr" +require "net/http" +require "socket" class Upload < ApplicationRecord include PgSearch::Model @@ -96,64 +96,76 @@ def rename!(new_filename) blob.update!(filename: sanitized) end - # Validate that a URL is safe to fetch (not targeting internal networks) - def self.assert_public_url!(url) - uri = URI.parse(url) + MAX_FETCH_REDIRECTS = 5 + + BLOCKED_FETCH_RANGES = %w[ + 0.0.0.0/8 10.0.0.0/8 100.64.0.0/10 127.0.0.0/8 169.254.0.0/16 172.16.0.0/12 + 192.0.0.0/24 192.168.0.0/16 198.18.0.0/15 224.0.0.0/4 240.0.0.0/4 + ::/128 ::1/128 64:ff9b::/96 fc00::/7 fe80::/10 + ].map { |range| IPAddr.new(range) }.freeze + def self.resolve_fetchable_addresses!(uri) unless %w[http https].include?(uri.scheme&.downcase) raise ArgumentError, "URL scheme must be http or https." end - host = uri.host - raise ArgumentError, "Invalid host" if host.nil? || host.empty? + host = uri.hostname + raise ArgumentError, "Invalid host" if host.blank? + + port = uri.port || uri.default_port - begin - addrs = Resolv.getaddresses(host) - rescue Resolv::ResolvError + addresses = begin - IPAddr.new(host) - addrs = [ host ] - rescue IPAddr::InvalidAddressError - raise ArgumentError, "Couldn't resolve host." + Addrinfo.getaddrinfo(host, port, nil, :STREAM).map(&:ip_address).uniq + rescue SocketError + [] end - end - raise ArgumentError, "Couldn't resolve host." if addrs.empty? + raise ArgumentError, "Couldn't resolve host." if addresses.empty? - addrs.each do |addr_str| - ip = IPAddr.new(addr_str) - raise ArgumentError, "IP address not allowed" if ip.loopback? || ip.private? || ip.link_local? + addresses.each do |address| + raise ArgumentError, "IP address not allowed" if blocked_fetch_address?(address) end - end - # Create upload from URL (for API/rescue operations) - # Checks quota via HEAD request before downloading when possible - def self.create_from_url(url, user:, provenance:, original_url: nil, authorization: nil, filename: nil) - assert_public_url!(url) - - redirect_validator = proc do |_response_env, new_request_env| - assert_public_url!(new_request_env[:url].to_s) - end + addresses + end - conn = build_http_client(redirect_validator) + def self.fetch_public_url!(url, max_bytes:, authorization: nil) + uri = URI.parse(url) + token = authorization + hops = 0 - headers = {} - headers["Authorization"] = authorization if authorization.present? + loop do + address = resolve_fetchable_addresses!(uri).first + result = perform_fetch(uri, address, token, max_bytes) + return result unless result.key?(:location) - # Pre-check file size via HEAD if possible - pre_check_quota_via_head(conn, url, headers, user) + hops += 1 + raise "Failed to download: too many redirects" if hops > MAX_FETCH_REDIRECTS + raise "Failed to download: redirect without a location" if result[:location].blank? - # Download the file - response = conn.get(url, nil, headers) - if response.status.between?(300, 399) - location = response.headers["location"] - raise "Failed to download: #{response.status} redirect to #{location}" + following = uri.merge(result[:location]) + token = nil unless same_fetch_origin?(uri, following) + uri = following end - raise "Failed to download: #{response.status}" unless response.success? + end + + # Create upload from URL (for API/rescue operations) + def self.create_from_url(url, user:, provenance:, original_url: nil, authorization: nil, filename: nil) + quota_service = QuotaService.new(user) + policy = quota_service.current_policy + remaining_storage = policy.max_total_storage - user.total_storage_bytes + raise "File would exceed storage quota" if remaining_storage <= 0 + + fetched = fetch_public_url!( + url, + max_bytes: [ policy.max_file_size, remaining_storage ].min, + authorization: authorization + ) filename ||= extract_filename_from_url(url) - body = response.body + body = fetched[:body] content_type = Marcel::MimeType.for(StringIO.new(body), name: filename) || - response.headers["content-type"] || + fetched[:content_type] || "application/octet-stream" content_type = normalize_content_type(content_type) @@ -182,37 +194,46 @@ def self.create_from_url(url, user:, provenance:, original_url: nil, authorizati class << self private - def build_http_client(redirect_validator = nil) - Faraday.new(ssl: { verify: true, verify_mode: OpenSSL::SSL::VERIFY_PEER }) do |f| - f.response :follow_redirects, limit: 5, callback: redirect_validator - f.adapter Faraday.default_adapter - end.tap do |conn| - conn.options.open_timeout = 30 - conn.options.timeout = 120 - end + def blocked_fetch_address?(address) + ip = IPAddr.new(address.split("%").first) + ip = ip.native if ip.ipv6? && ip.ipv4_mapped? + BLOCKED_FETCH_RANGES.any? { |range| range.family == ip.family && range.include?(ip) } + rescue IPAddr::InvalidAddressError + true end - def pre_check_quota_via_head(conn, url, headers, user) - head_response = conn.head(url, nil, headers) - return unless head_response.success? - - content_length = head_response.headers["content-length"]&.to_i - return unless content_length && content_length > 0 - - quota_service = QuotaService.new(user) - policy = quota_service.current_policy - - if content_length > policy.max_file_size - raise "File too large: #{ActiveSupport::NumberHelper.number_to_human_size(content_length)} " \ - "exceeds limit of #{ActiveSupport::NumberHelper.number_to_human_size(policy.max_file_size)}" + def perform_fetch(uri, address, authorization, max_bytes) + http = Net::HTTP.new(uri.hostname, uri.port) + http.ipaddr = address + http.use_ssl = uri.scheme == "https" + http.verify_mode = OpenSSL::SSL::VERIFY_PEER + http.open_timeout = 30 + http.read_timeout = 120 + + request = Net::HTTP::Get.new(uri.request_uri) + request["Authorization"] = authorization if authorization.present? + + http.start do |connection| + connection.request(request) do |response| + return { location: response["location"] } if response.is_a?(Net::HTTPRedirection) + raise "Failed to download: #{response.code}" unless response.is_a?(Net::HTTPSuccess) + + body = +"" + response.read_body do |chunk| + body << chunk + if body.bytesize > max_bytes + raise "File too large: exceeds limit of " \ + "#{ActiveSupport::NumberHelper.number_to_human_size(max_bytes)}" + end + end + + return { body: body, content_type: response["content-type"] } + end end + end - return if quota_service.can_upload?(content_length) - - raise "File would exceed storage quota" - rescue Faraday::Error - # HEAD request failed — proceed with GET and check after - nil + def same_fetch_origin?(from, to) + [ from.scheme, from.hostname, from.port ] == [ to.scheme, to.hostname, to.port ] end def extract_filename_from_url(url) diff --git a/test/controllers/api/v4/uploads_controller_test.rb b/test/controllers/api/v4/uploads_controller_test.rb index e9ec798..32c9326 100644 --- a/test/controllers/api/v4/uploads_controller_test.rb +++ b/test/controllers/api/v4/uploads_controller_test.rb @@ -46,20 +46,10 @@ class API::V4::UploadsControllerTest < ActionDispatch::IntegrationTest test "should upload from URL with valid token" do url = "https://example.com/test.jpg" - fake_response = Struct.new(:status, :body, :headers) { def success? = true } - .new(200, "fake image data", { "content-type" => "image/jpeg" }) - fake_opts = Object.new.tap { |o| o.define_singleton_method(:open_timeout=) { |_| } - o.define_singleton_method(:timeout=) { |_| } } - fake_head_response = Struct.new(:status, :body, :headers) { def success? = false } - .new(405, "", {}) - fake_conn = Object.new.tap { |c| c.define_singleton_method(:options) { fake_opts } - c.define_singleton_method(:get) { |*| fake_response } - c.define_singleton_method(:head) { |*| fake_head_response } } - - original_faraday_new = Faraday.method(:new) - original_assert_public_url = Upload.method(:assert_public_url!) - Faraday.define_singleton_method(:new) { |*, **, &_block| fake_conn } - Upload.define_singleton_method(:assert_public_url!) { |_url| nil } + original_fetch = Upload.method(:fetch_public_url!) + Upload.define_singleton_method(:fetch_public_url!) do |_url, **_options| + { body: "fake image data", content_type: "image/jpeg" } + end begin assert_difference("Upload.count", 1) do post api_v4_upload_from_url_url, @@ -70,8 +60,7 @@ class API::V4::UploadsControllerTest < ActionDispatch::IntegrationTest } end ensure - Faraday.define_singleton_method(:new, original_faraday_new) - Upload.define_singleton_method(:assert_public_url!, original_assert_public_url) + Upload.define_singleton_method(:fetch_public_url!, original_fetch) end assert_response :created @@ -96,15 +85,10 @@ class API::V4::UploadsControllerTest < ActionDispatch::IntegrationTest test "should handle upload errors gracefully" do url = "https://example.com/broken.jpg" - fake_opts = Object.new.tap { |o| o.define_singleton_method(:open_timeout=) { |_| } - o.define_singleton_method(:timeout=) { |_| } } - fake_conn = Object.new.tap { |c| c.define_singleton_method(:options) { fake_opts } - c.define_singleton_method(:get) { |*| raise StandardError, "Network error" } } - - original_faraday_new = Faraday.method(:new) - original_assert_public_url = Upload.method(:assert_public_url!) - Faraday.define_singleton_method(:new) { |*, **, &_block| fake_conn } - Upload.define_singleton_method(:assert_public_url!) { |_url| nil } + original_fetch = Upload.method(:fetch_public_url!) + Upload.define_singleton_method(:fetch_public_url!) do |_url, **_options| + raise StandardError, "Network error" + end begin post api_v4_upload_from_url_url, params: { url: url }.to_json, @@ -113,8 +97,7 @@ class API::V4::UploadsControllerTest < ActionDispatch::IntegrationTest "Content-Type" => "application/json" } ensure - Faraday.define_singleton_method(:new, original_faraday_new) - Upload.define_singleton_method(:assert_public_url!, original_assert_public_url) + Upload.define_singleton_method(:fetch_public_url!, original_fetch) end assert_response :unprocessable_entity From bee0ff2830b3b61f222417ef1ea1ab2a146ed150 Mon Sep 17 00:00:00 2001 From: "Abdallah Ebrahim (dracula)" Date: Wed, 19 Aug 2026 20:30:44 +0300 Subject: [PATCH 2/4] fix: rate limits --- app/controllers/api/v4/uploads_controller.rb | 4 ++++ app/models/upload.rb | 15 +++++++++++---- 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/app/controllers/api/v4/uploads_controller.rb b/app/controllers/api/v4/uploads_controller.rb index 3315266..a7ea8a0 100644 --- a/app/controllers/api/v4/uploads_controller.rb +++ b/app/controllers/api/v4/uploads_controller.rb @@ -5,6 +5,10 @@ module V4 class UploadsController < ApplicationController before_action :check_quota, only: [ :create, :create_from_url ] + rate_limit to: 10, within: 1.minute, only: :create_from_url, + by: -> { current_token&.id }, + with: -> { render json: { error: "Too many requests" }, status: :too_many_requests } + # POST /api/v4/upload def create file = params[:file] diff --git a/app/models/upload.rb b/app/models/upload.rb index 39b88ef..1a9cb33 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -97,6 +97,9 @@ def rename!(new_filename) end MAX_FETCH_REDIRECTS = 5 + FETCH_OPEN_TIMEOUT = 5 + FETCH_READ_TIMEOUT = 30 + FETCH_DEADLINE = 60 BLOCKED_FETCH_RANGES = %w[ 0.0.0.0/8 10.0.0.0/8 100.64.0.0/10 127.0.0.0/8 169.254.0.0/16 172.16.0.0/12 @@ -133,10 +136,11 @@ def self.fetch_public_url!(url, max_bytes:, authorization: nil) uri = URI.parse(url) token = authorization hops = 0 + deadline = Process.clock_gettime(Process::CLOCK_MONOTONIC) + FETCH_DEADLINE loop do address = resolve_fetchable_addresses!(uri).first - result = perform_fetch(uri, address, token, max_bytes) + result = perform_fetch(uri, address, token, max_bytes, deadline) return result unless result.key?(:location) hops += 1 @@ -202,13 +206,13 @@ def blocked_fetch_address?(address) true end - def perform_fetch(uri, address, authorization, max_bytes) + def perform_fetch(uri, address, authorization, max_bytes, deadline) http = Net::HTTP.new(uri.hostname, uri.port) http.ipaddr = address http.use_ssl = uri.scheme == "https" http.verify_mode = OpenSSL::SSL::VERIFY_PEER - http.open_timeout = 30 - http.read_timeout = 120 + http.open_timeout = FETCH_OPEN_TIMEOUT + http.read_timeout = FETCH_READ_TIMEOUT request = Net::HTTP::Get.new(uri.request_uri) request["Authorization"] = authorization if authorization.present? @@ -225,6 +229,9 @@ def perform_fetch(uri, address, authorization, max_bytes) raise "File too large: exceeds limit of " \ "#{ActiveSupport::NumberHelper.number_to_human_size(max_bytes)}" end + if Process.clock_gettime(Process::CLOCK_MONOTONIC) > deadline + raise "Failed to download: timed out" + end end return { body: body, content_type: response["content-type"] } From de5263545fef52893305f2a4012e574b7e276900 Mon Sep 17 00:00:00 2001 From: "Abdallah Ebrahim (dracula)" Date: Wed, 19 Aug 2026 20:33:34 +0300 Subject: [PATCH 3/4] fix: reset session on sign-in --- app/controllers/sessions_controller.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/controllers/sessions_controller.rb b/app/controllers/sessions_controller.rb index a6ce6c4..821b353 100644 --- a/app/controllers/sessions_controller.rb +++ b/app/controllers/sessions_controller.rb @@ -6,6 +6,7 @@ class SessionsController < ApplicationController def create auth = request.env["omniauth.auth"] user = User.find_or_create_from_omniauth(auth) + reset_session session[:user_id] = user.id # Check and upgrade verification status if needed From 9c06a7d837bbe84d86dbc8d47342c6a2fb568427 Mon Sep 17 00:00:00 2001 From: Echo Date: Sun, 30 Aug 2026 11:11:55 -0400 Subject: [PATCH 4/4] rate limit per user --- app/controllers/api/v4/uploads_controller.rb | 2 +- .../api/v4/uploads_controller_test.rb | 41 +++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/app/controllers/api/v4/uploads_controller.rb b/app/controllers/api/v4/uploads_controller.rb index a7ea8a0..5dd24b2 100644 --- a/app/controllers/api/v4/uploads_controller.rb +++ b/app/controllers/api/v4/uploads_controller.rb @@ -6,7 +6,7 @@ class UploadsController < ApplicationController before_action :check_quota, only: [ :create, :create_from_url ] rate_limit to: 10, within: 1.minute, only: :create_from_url, - by: -> { current_token&.id }, + by: -> { current_user&.id }, with: -> { render json: { error: "Too many requests" }, status: :too_many_requests } # POST /api/v4/upload diff --git a/test/controllers/api/v4/uploads_controller_test.rb b/test/controllers/api/v4/uploads_controller_test.rb index 32c9326..bf4051e 100644 --- a/test/controllers/api/v4/uploads_controller_test.rb +++ b/test/controllers/api/v4/uploads_controller_test.rb @@ -105,6 +105,47 @@ class API::V4::UploadsControllerTest < ActionDispatch::IntegrationTest assert json["error"].include?("Upload failed") end + test "should rate limit URL uploads per user across API keys" do + second_token = @user.api_keys.create!(name: "Second Test Key").token + upload = create_upload("rate-limit.jpg") + requests_by_key = Hash.new(0) + rate_limit_store = API::V4::UploadsController.cache_store + original_create_from_url = Upload.method(:create_from_url) + original_increment = rate_limit_store.method(:increment) + + Upload.define_singleton_method(:create_from_url) { |_url, **_options| upload } + rate_limit_store.define_singleton_method(:increment) do |key, amount, **_options| + requests_by_key[key] += amount + end + + begin + 10.times do |index| + post api_v4_upload_from_url_url, + params: { url: "https://example.com/rate-limit.jpg" }.to_json, + headers: { + "Authorization" => "Bearer #{index.even? ? @token : second_token}", + "Content-Type" => "application/json" + } + + assert_response :created + end + + post api_v4_upload_from_url_url, + params: { url: "https://example.com/rate-limit.jpg" }.to_json, + headers: { + "Authorization" => "Bearer #{second_token}", + "Content-Type" => "application/json" + } + ensure + Upload.define_singleton_method(:create_from_url, original_create_from_url) + rate_limit_store.define_singleton_method(:increment, original_increment) + end + + assert_response :too_many_requests + assert_equal({ "error" => "Too many requests" }, JSON.parse(response.body)) + assert_equal [ "rate-limit:api/v4/uploads:#{@user.id}" ], requests_by_key.keys + end + # --- batch upload --- test "should upload a batch of files" do