fix(ssrf): close safe fetch tempfiles explicitly
This commit is contained in:
@@ -8,11 +8,13 @@ class Messages::Messenger::MessageBuilder
|
||||
|
||||
params = attachment_params(attachment)
|
||||
attachment_obj = @message.attachments.new(params.except(:remote_file_url))
|
||||
downloaded_file = nil
|
||||
if facebook_reel?(attachment)
|
||||
attachment_obj.save!
|
||||
update_facebook_reel_content(attachment)
|
||||
elsif params[:remote_file_url]
|
||||
return unless attach_file(attachment_obj, params[:remote_file_url])
|
||||
downloaded_file = attach_file(attachment_obj, params[:remote_file_url])
|
||||
return unless downloaded_file
|
||||
|
||||
attachment_obj.save!
|
||||
else
|
||||
@@ -22,24 +24,28 @@ class Messages::Messenger::MessageBuilder
|
||||
fetch_ig_story_link(attachment_obj) if attachment_obj.file_type == 'ig_story'
|
||||
fetch_ig_post_link(attachment_obj) if attachment_obj.file_type == 'ig_post'
|
||||
update_attachment_file_type(attachment_obj)
|
||||
ensure
|
||||
downloaded_file&.close!
|
||||
end
|
||||
# rubocop:enable Metrics/CyclomaticComplexity, Metrics/PerceivedComplexity
|
||||
|
||||
def attach_file(attachment, file_url)
|
||||
downloaded_file = nil
|
||||
SafeFetch.fetch(
|
||||
file_url,
|
||||
allowed_content_types: Attachment::ACCEPTABLE_FILE_TYPES
|
||||
) do |attachment_file|
|
||||
downloaded_file = attachment_file
|
||||
attachment.file.attach(
|
||||
io: attachment_file.tempfile,
|
||||
filename: attachment_file.original_filename,
|
||||
content_type: attachment_file.content_type
|
||||
)
|
||||
end
|
||||
true
|
||||
downloaded_file
|
||||
rescue SafeFetch::Error => e
|
||||
Rails.logger.info "Error downloading Messenger attachment from #{file_url}: #{e.message}: Skipping"
|
||||
false
|
||||
nil
|
||||
end
|
||||
|
||||
def attachment_params(attachment)
|
||||
|
||||
@@ -21,6 +21,8 @@ class Api::V1::Accounts::UploadController < Api::V1::Accounts::BaseController
|
||||
def create_from_url
|
||||
SafeFetch.fetch(params[:external_url].to_s) do |result|
|
||||
create_and_save_blob(result.tempfile, result.filename, result.content_type)
|
||||
ensure
|
||||
result.close!
|
||||
end
|
||||
rescue SafeFetch::HttpError => e
|
||||
render_error(I18n.t('errors.upload.fetch_failed_with_message', message: e.message), :unprocessable_entity)
|
||||
|
||||
@@ -40,6 +40,8 @@ class Avatar::AvatarFromUrlJob < ApplicationJob
|
||||
allowed_content_types: %w[image/jpeg image/png image/gif]
|
||||
) do |avatar_file|
|
||||
attach_avatar(avatarable, avatar_file)
|
||||
ensure
|
||||
avatar_file.close!
|
||||
end
|
||||
end
|
||||
|
||||
|
||||
@@ -4,6 +4,7 @@ class Sms::IncomingMessageService
|
||||
pattr_initialize [:inbox!, :params!]
|
||||
|
||||
def perform
|
||||
@downloaded_files = []
|
||||
set_contact
|
||||
set_conversation
|
||||
@message = @conversation.messages.create!(
|
||||
@@ -16,6 +17,8 @@ class Sms::IncomingMessageService
|
||||
)
|
||||
attach_files
|
||||
@message.save!
|
||||
ensure
|
||||
close_downloaded_files
|
||||
end
|
||||
|
||||
private
|
||||
@@ -84,6 +87,7 @@ class Sms::IncomingMessageService
|
||||
next if media_url.end_with?('.smil', '.xml')
|
||||
|
||||
download_attachment_file(media_url) do |attachment_file|
|
||||
track_downloaded_file(attachment_file)
|
||||
@message.attachments.new(
|
||||
account_id: @message.account_id,
|
||||
file_type: file_type(attachment_file.content_type),
|
||||
@@ -107,4 +111,12 @@ class Sms::IncomingMessageService
|
||||
rescue SafeFetch::Error => e
|
||||
Rails.logger.info "Error downloading SMS attachment from #{media_url}: #{e.message}: Skipping"
|
||||
end
|
||||
|
||||
def track_downloaded_file(attachment_file)
|
||||
@downloaded_files << attachment_file
|
||||
end
|
||||
|
||||
def close_downloaded_files
|
||||
Array(@downloaded_files).each(&:close!)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -8,6 +8,7 @@ class Telegram::IncomingMessageService
|
||||
pattr_initialize [:inbox!, :params!]
|
||||
|
||||
def perform
|
||||
@downloaded_files = []
|
||||
# chatwoot doesn't support group conversations at the moment
|
||||
transform_business_message!
|
||||
return unless private_message?
|
||||
@@ -34,6 +35,8 @@ class Telegram::IncomingMessageService
|
||||
|
||||
process_message_attachments if message_params?
|
||||
@message.save!
|
||||
ensure
|
||||
close_downloaded_files
|
||||
end
|
||||
|
||||
private
|
||||
@@ -167,6 +170,7 @@ class Telegram::IncomingMessageService
|
||||
end
|
||||
|
||||
def build_file_attachment(attachment_file)
|
||||
track_downloaded_file(attachment_file)
|
||||
@message.attachments.new(
|
||||
account_id: @message.account_id,
|
||||
file_type: file_content_type,
|
||||
@@ -233,6 +237,14 @@ class Telegram::IncomingMessageService
|
||||
params[:message][:video_note].presence
|
||||
end
|
||||
|
||||
def track_downloaded_file(attachment_file)
|
||||
@downloaded_files << attachment_file
|
||||
end
|
||||
|
||||
def close_downloaded_files
|
||||
Array(@downloaded_files).each(&:close!)
|
||||
end
|
||||
|
||||
def transform_business_message!
|
||||
params[:message] = params[:business_message] if params[:business_message] && !params[:message]
|
||||
end
|
||||
|
||||
@@ -32,6 +32,7 @@ class Tiktok::MessageService
|
||||
end
|
||||
|
||||
def create_message
|
||||
@downloaded_files = []
|
||||
message = conversation.messages.build(
|
||||
content: message_content,
|
||||
account_id: channel.inbox.account_id,
|
||||
@@ -48,6 +49,8 @@ class Tiktok::MessageService
|
||||
|
||||
create_message_attachments(message)
|
||||
message.save!
|
||||
ensure
|
||||
close_downloaded_files
|
||||
end
|
||||
|
||||
def message_content
|
||||
@@ -65,6 +68,7 @@ class Tiktok::MessageService
|
||||
return unless image_message?
|
||||
|
||||
fetch_attachment(channel, tt_conversation_id, tt_message_id, tt_image_media_id) do |attachment_file|
|
||||
track_downloaded_file(attachment_file)
|
||||
message.attachments.new(
|
||||
account_id: message.account_id,
|
||||
file_type: :image,
|
||||
@@ -176,4 +180,12 @@ class Tiktok::MessageService
|
||||
def outgoing_message?
|
||||
!incoming_message?
|
||||
end
|
||||
|
||||
def track_downloaded_file(attachment_file)
|
||||
@downloaded_files << attachment_file
|
||||
end
|
||||
|
||||
def close_downloaded_files
|
||||
Array(@downloaded_files).each(&:close!)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -5,6 +5,7 @@ class Twilio::IncomingMessageService
|
||||
pattr_initialize [:params!]
|
||||
|
||||
def perform
|
||||
@downloaded_files = []
|
||||
return if twilio_channel.blank?
|
||||
|
||||
set_contact
|
||||
@@ -20,6 +21,8 @@ class Twilio::IncomingMessageService
|
||||
attach_files
|
||||
attach_location if location_message?
|
||||
@message.save!
|
||||
ensure
|
||||
close_downloaded_files
|
||||
end
|
||||
|
||||
private
|
||||
@@ -145,6 +148,7 @@ class Twilio::IncomingMessageService
|
||||
|
||||
def attach_single_file(media_url)
|
||||
download_attachment_file(media_url) do |attachment_file|
|
||||
track_downloaded_file(attachment_file)
|
||||
@message.attachments.new(
|
||||
account_id: @message.account_id,
|
||||
file_type: file_type(attachment_file.content_type),
|
||||
@@ -218,5 +222,13 @@ class Twilio::IncomingMessageService
|
||||
def contact_name_matches_phone_number?
|
||||
@contact.name == phone_number || @contact.name == formatted_phone_number
|
||||
end
|
||||
|
||||
def track_downloaded_file(attachment_file)
|
||||
@downloaded_files << attachment_file
|
||||
end
|
||||
|
||||
def close_downloaded_files
|
||||
Array(@downloaded_files).each(&:close!)
|
||||
end
|
||||
end
|
||||
# rubocop:enable Metrics/ClassLength
|
||||
|
||||
@@ -24,6 +24,8 @@ class Whatsapp::IncomingMessageBaseService
|
||||
private
|
||||
|
||||
def process_messages
|
||||
@downloaded_files = []
|
||||
|
||||
# We don't support reactions & ephemeral message now, we need to skip processing the message
|
||||
# if the webhook event is a reaction or an ephermal message or an unsupported message.
|
||||
return if unprocessable_message_type?(message_type)
|
||||
@@ -43,6 +45,8 @@ class Whatsapp::IncomingMessageBaseService
|
||||
set_conversation
|
||||
create_messages
|
||||
end
|
||||
ensure
|
||||
close_downloaded_files
|
||||
end
|
||||
|
||||
def process_statuses
|
||||
@@ -149,6 +153,7 @@ class Whatsapp::IncomingMessageBaseService
|
||||
@message.content ||= attachment_payload[:caption]
|
||||
|
||||
download_attachment_file(attachment_payload) do |attachment_file|
|
||||
track_downloaded_file(attachment_file)
|
||||
@message.attachments.new(
|
||||
account_id: @message.account_id,
|
||||
file_type: file_content_type(message_type),
|
||||
@@ -229,4 +234,12 @@ class Whatsapp::IncomingMessageBaseService
|
||||
formatted_phone_number = TelephoneNumber.parse(phone_number).international_number
|
||||
@contact.name == phone_number || @contact.name == formatted_phone_number
|
||||
end
|
||||
|
||||
def track_downloaded_file(attachment_file)
|
||||
@downloaded_files << attachment_file
|
||||
end
|
||||
|
||||
def close_downloaded_files
|
||||
Array(@downloaded_files).each(&:close!)
|
||||
end
|
||||
end
|
||||
|
||||
@@ -43,6 +43,8 @@ class Captain::Tools::HttpTool < Agents::Tool
|
||||
validate_content_type: false
|
||||
) do |response|
|
||||
response_body = response.tempfile.read
|
||||
ensure
|
||||
response.close!
|
||||
end
|
||||
|
||||
response_body
|
||||
|
||||
Reference in New Issue
Block a user