diff --git a/app/mailboxes/imap/imap_mailbox.rb b/app/mailboxes/imap/imap_mailbox.rb index 27bc88e06..25b8f4d85 100644 --- a/app/mailboxes/imap/imap_mailbox.rb +++ b/app/mailboxes/imap/imap_mailbox.rb @@ -65,11 +65,10 @@ class Imap::ImapMailbox end def in_reply_to - @processed_mail.in_reply_to + sanitize_mailbox_value(@processed_mail.in_reply_to) end def find_conversation_by_references - references = Array.wrap(@inbound_mail.references) references.each do |message_id| match = FALLBACK_CONVERSATION_PATTERN.match(message_id) @@ -80,8 +79,6 @@ class Imap::ImapMailbox def find_message_by_references message_to_return = nil - references = Array.wrap(@inbound_mail.references) - references.each do |message_id| message = @inbox.messages.find_by(source_id: message_id) message_to_return = message if message.present? @@ -100,7 +97,7 @@ class Imap::ImapMailbox source: 'email', in_reply_to: in_reply_to, auto_reply: @processed_mail.auto_reply?, - mail_subject: @processed_mail.subject, + mail_subject: sanitize_mailbox_value(@processed_mail.subject), initiated_at: { timestamp: Time.now.utc } @@ -110,7 +107,7 @@ class Imap::ImapMailbox end def find_or_create_contact - @contact = @inbox.contacts.from_email(@processed_mail.original_sender) + @contact = @inbox.contacts.from_email(original_sender_email) if @contact.present? @contact_inbox = ContactInbox.find_by(inbox: @inbox, contact: @contact) else @@ -119,6 +116,14 @@ class Imap::ImapMailbox end def identify_contact_name - processed_mail.sender_name || processed_mail.from.first.split('@').first + sanitize_mailbox_value(processed_mail.sender_name || processed_mail.from.first.split('@').first) + end + + def original_sender_email + sanitize_mailbox_value(@processed_mail.original_sender) + end + + def references + sanitize_mailbox_value(Array.wrap(@inbound_mail.references)) end end diff --git a/app/mailboxes/mailbox_helper.rb b/app/mailboxes/mailbox_helper.rb index edabfd4ce..fd3e0fb16 100644 --- a/app/mailboxes/mailbox_helper.rb +++ b/app/mailboxes/mailbox_helper.rb @@ -1,27 +1,16 @@ module MailboxHelper include MailboxInlineAttachmentHelper + include MailboxSanitizer include ::FileTypeHelper private def create_message Rails.logger.info "[MailboxHelper] Creating message #{processed_mail.message_id}" - return if @conversation.messages.find_by(source_id: processed_mail.message_id).present? + source_id = sanitize_mailbox_value(processed_mail.message_id) + return if @conversation.messages.find_by(source_id: source_id).present? - @message = @conversation.messages.create!( - account_id: @conversation.account_id, - sender: @conversation.contact, - content: mail_content&.truncate(150_000), - inbox_id: @conversation.inbox_id, - message_type: 'incoming', - content_type: 'incoming_email', - source_id: processed_mail.message_id, - content_attributes: { - email: processed_mail.serialized_data, - cc_email: processed_mail.cc, - bcc_email: processed_mail.bcc - } - ) + @message = @conversation.messages.create!(sanitized_message_attributes(source_id)) end def add_attachments_to_message @@ -101,13 +90,16 @@ module MailboxHelper end def create_contact + sender_email = sanitize_mailbox_value(processed_mail.original_sender) + message_id = sanitize_mailbox_value(processed_mail.message_id) + @contact_inbox = ::ContactInboxWithContactBuilder.new( - source_id: processed_mail.original_sender, + source_id: sender_email, inbox: @inbox, contact_attributes: { - name: identify_contact_name, - email: processed_mail.original_sender, - additional_attributes: { source_id: "email:#{processed_mail.message_id}" } + name: sanitize_mailbox_value(identify_contact_name), + email: sender_email, + additional_attributes: { source_id: "email:#{message_id}" } } ).perform diff --git a/app/mailboxes/mailbox_sanitizer.rb b/app/mailboxes/mailbox_sanitizer.rb new file mode 100644 index 000000000..b78bb1258 --- /dev/null +++ b/app/mailboxes/mailbox_sanitizer.rb @@ -0,0 +1,34 @@ +module MailboxSanitizer + NULL_BYTE = "\u0000".freeze + + private + + def sanitized_message_attributes(source_id) + { + account_id: @conversation.account_id, + sender: @conversation.contact, + content: sanitize_mailbox_value(mail_content)&.truncate(150_000), + inbox_id: @conversation.inbox_id, + message_type: 'incoming', + content_type: 'incoming_email', + source_id: source_id, + content_attributes: sanitized_content_attributes + } + end + + def sanitized_content_attributes + sanitize_mailbox_value( + email: processed_mail.serialized_data, + cc_email: processed_mail.cc, + bcc_email: processed_mail.bcc + ) + end + + def sanitize_mailbox_value(value) + return value.delete(NULL_BYTE) if value.is_a?(String) + return value.map { |item| sanitize_mailbox_value(item) } if value.is_a?(Array) + return value.transform_values { |item| sanitize_mailbox_value(item) } if value.is_a?(Hash) + + value + end +end diff --git a/app/services/mailbox/conversation_finder_strategies/base_strategy.rb b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb index c1e738805..fd61fd9e0 100644 --- a/app/services/mailbox/conversation_finder_strategies/base_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/base_strategy.rb @@ -1,4 +1,6 @@ class Mailbox::ConversationFinderStrategies::BaseStrategy + include MailboxSanitizer + attr_reader :mail def initialize(mail) diff --git a/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb index b14f851f5..e3d8270f1 100644 --- a/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/in_reply_to_strategy.rb @@ -14,7 +14,7 @@ class Mailbox::ConversationFinderStrategies::InReplyToStrategy < Mailbox::Conver def find return nil if mail.in_reply_to.blank? - in_reply_to_addresses = Array.wrap(mail.in_reply_to) + in_reply_to_addresses = sanitize_mailbox_value(Array.wrap(mail.in_reply_to)) in_reply_to_addresses.each do |in_reply_to| # Try extracting UUID from patterns diff --git a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb index a21fcebe5..f9f5c3cf8 100644 --- a/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/new_conversation_strategy.rb @@ -45,11 +45,11 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: end def original_sender_email - @processed_mail.original_sender&.downcase + sanitize_mailbox_value(@processed_mail.original_sender)&.downcase end def identify_contact_name - @processed_mail.sender_name || @processed_mail.from.first.split('@').first + sanitize_mailbox_value(@processed_mail.sender_name || @processed_mail.from.first.split('@').first) end def build_conversation @@ -63,7 +63,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: in_reply_to: in_reply_to, source: 'email', auto_reply: @processed_mail.auto_reply?, - mail_subject: @processed_mail.subject, + mail_subject: sanitize_mailbox_value(@processed_mail.subject), initiated_at: { timestamp: Time.now.utc } @@ -72,7 +72,7 @@ class Mailbox::ConversationFinderStrategies::NewConversationStrategy < Mailbox:: end def in_reply_to - mail['In-Reply-To'].try(:value) + sanitize_mailbox_value(mail['In-Reply-To'].try(:value)) end def find_conversation_by_in_reply_to diff --git a/app/services/mailbox/conversation_finder_strategies/references_strategy.rb b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb index 86a0aa3c5..4420c5c7d 100644 --- a/app/services/mailbox/conversation_finder_strategies/references_strategy.rb +++ b/app/services/mailbox/conversation_finder_strategies/references_strategy.rb @@ -21,7 +21,7 @@ class Mailbox::ConversationFinderStrategies::ReferencesStrategy < Mailbox::Conve return nil if mail.references.blank? return nil unless @channel # No valid channel found - references = Array.wrap(mail.references) + references = sanitize_mailbox_value(Array.wrap(mail.references)) references.each do |reference| conversation = find_conversation_from_reference(reference) diff --git a/spec/mailboxes/imap/imap_mailbox_spec.rb b/spec/mailboxes/imap/imap_mailbox_spec.rb index 309a38a65..9a6797aaf 100644 --- a/spec/mailboxes/imap/imap_mailbox_spec.rb +++ b/spec/mailboxes/imap/imap_mailbox_spec.rb @@ -99,6 +99,33 @@ RSpec.describe Imap::ImapMailbox do end end + context 'when a new email contains null bytes' do + let(:inbound_mail) do + Mail.new.tap do |mail| + mail.from = 'email@gmail.com' + mail.to = 'imap@gmail.com' + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail['In-Reply-To'] = "source\u0000@example.com" + mail.references = ["reference\u0000@example.com"] + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + end + end + + it 'creates sanitized conversation and message records' do + expect { class_instance.process(inbound_mail, channel) }.to change(Conversation, :count).by(1) + + message = conversation.messages.last + + expect(conversation.additional_attributes['in_reply_to']).to eq('source@example.com') + expect(conversation.additional_attributes['mail_subject']).to eq('Hello') + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end + context 'when a new email with invalid from' do let(:inbound_mail) { create_inbound_email_from_mail(from: 'invalidemail', to: 'imap@gmail.com', subject: 'Hello!') } diff --git a/spec/mailboxes/mailbox_helper_spec.rb b/spec/mailboxes/mailbox_helper_spec.rb index 613040cb3..4b02205ec 100644 --- a/spec/mailboxes/mailbox_helper_spec.rb +++ b/spec/mailboxes/mailbox_helper_spec.rb @@ -47,6 +47,31 @@ RSpec.describe MailboxHelper do helper_instance.send(:create_message) end end + + context 'when message data contains null bytes' do + let(:mail) do + mail = Mail.new + mail.from = 'Sender ' + mail.to = 'Inbox ' + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + mail + end + + it 'creates the message with sanitized values' do + helper_instance = mailbox_helper_obj.new(conversation, processed_mail) + + expect { helper_instance.send(:create_message) }.to change(conversation.messages, :count).by(1) + + message = conversation.messages.last + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.dig('email', 'message_id')).to eq('message@example.com') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end end describe '#embed_plain_text_email_with_inline_image' do diff --git a/spec/mailboxes/reply_mailbox_spec.rb b/spec/mailboxes/reply_mailbox_spec.rb index d062c7d73..20ce60dad 100644 --- a/spec/mailboxes/reply_mailbox_spec.rb +++ b/spec/mailboxes/reply_mailbox_spec.rb @@ -67,6 +67,40 @@ RSpec.describe ReplyMailbox do end end + context 'when new conversation email contains null bytes' do + let(:email_channel) { create(:channel_email, email: 'test@example.com', account: account) } + let(:null_byte_mail) { create_inbound_email_from_mail(from: 'sender@example.com', to: email_channel.email, subject: 'Hello') } + let(:mail_with_null_bytes) do + Mail.new.tap do |mail| + mail.from = 'sender@example.com' + mail.to = email_channel.email + mail.subject = "Hello\u0000" + mail.message_id = "message\u0000@example.com" + mail['In-Reply-To'] = "source\u0000@example.com" + mail.references = ["reference\u0000@example.com"] + mail.content_type = 'text/plain' + mail.body = "Body\u0000 text" + end + end + + before do + allow(null_byte_mail).to receive(:mail).and_return(mail_with_null_bytes) + end + + it 'creates sanitized conversation and message records' do + expect { described_class.receive null_byte_mail }.to change(Conversation, :count).by(1) + + conversation = Conversation.last + message = conversation.messages.last + + expect(conversation.additional_attributes['in_reply_to']).to eq('source@example.com') + expect(conversation.additional_attributes['mail_subject']).to eq('Hello') + expect(message.source_id).to eq('message@example.com') + expect(message.content).to eq('Body text') + expect(message.content_attributes.to_json).not_to include('\u0000') + end + end + context 'with inline attachments' do let(:mail_with_inline_images) { create_inbound_email_from_fixture('mail_with_inline_images.eml') } let(:described_subject) { described_class.receive mail_with_inline_images }