2.6.x SMTP security: prevent command injection via To/From addresses

CHANGELOG.rdoc

@@ -1,3 +1,8 @@
+== Version 2.6.6 - unreleased
+
+Security:
+* #1097 – SMTP security: prevent command injection via To/From addresses. (jeremy)
+
 == Version 2.6.5 - 2017-04-26 Jeremy Daer <[email protected]>
 
 Features:

lib/mail/check_delivery_params.rb

@@ -1,21 +1,58 @@
 # frozen_string_literal: true
 module Mail
-  module CheckDeliveryParams
-    def check_delivery_params(mail)
-      if Utilities.blank?(mail.smtp_envelope_from)
-        raise ArgumentError.new('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+  module CheckDeliveryParams #:nodoc:
+    class << self
+      def check(mail)
+        [ check_from(mail.smtp_envelope_from),
+          check_to(mail.smtp_envelope_to),
+          check_message(mail) ]
       end
 
-      if Utilities.blank?(mail.smtp_envelope_to)
-        raise ArgumentError.new('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+      def check_from(addr)
+        if Utilities.blank?(addr)
+          raise ArgumentError, "SMTP From address may not be blank: #{addr.inspect}"
+        end
+
+        check_addr 'From', addr
+      end
+
+      def check_to(addrs)
+        if Utilities.blank?(addrs)
+          raise ArgumentError, "SMTP To address may not be blank: #{addrs.inspect}"
+        end
+
+        Array(addrs).map do |addr|
+          check_addr 'To', addr
+        end
       end
 
-      message = mail.encoded if mail.respond_to?(:encoded)
-      if Utilities.blank?(message)
-        raise ArgumentError.new('An encoded message is required to send an email')
+      def check_addr(addr_name, addr)
+        validate_smtp_addr addr do |error_message|
+          raise ArgumentError, "SMTP #{addr_name} address #{error_message}: #{addr.inspect}"
+        end
       end
 
-      [mail.smtp_envelope_from, mail.smtp_envelope_to, message]
+      def validate_smtp_addr(addr)
+        if addr.bytesize > 2048
+          yield 'may not exceed 2kB'
+        end
+
+        if /[\r\n]/ =~ addr
+          yield 'may not contain CR or LF line breaks'
+        end
+
+        addr
+      end
+
+      def check_message(message)
+        message = message.encoded if message.respond_to?(:encoded)
+
+        if Utilities.blank?(message)
+          raise ArgumentError, 'An encoded message is required to send an email'
+        end
+
+        message
+      end
     end
   end
 end

lib/mail/network/delivery_methods/file_delivery.rb

@@ -2,7 +2,6 @@
 require 'mail/check_delivery_params'
 
 module Mail
-
   # FileDelivery class delivers emails into multiple files based on the destination
   # address.  Each file is appended to if it already exists.
   # 
@@ -14,22 +13,20 @@ module Mail
   # Make sure the path you specify with :location is writable by the Ruby process
   # running Mail.
   class FileDelivery
-    include Mail::CheckDeliveryParams
-
     if RUBY_VERSION >= '1.9.1'
       require 'fileutils'
     else
       require 'ftools'
     end
 
+    attr_accessor :settings
+
     def initialize(values)
       self.settings = { :location => './mails' }.merge!(values)
     end
-
-    attr_accessor :settings
-
+
     def deliver!(mail)
-      check_delivery_params(mail)
+      Mail::CheckDeliveryParams.check(mail)
 
       if ::File.respond_to?(:makedirs)
         ::File.makedirs settings[:location]
@@ -41,6 +38,5 @@ def deliver!(mail)
         ::File.open(::File.join(settings[:location], File.basename(to.to_s)), 'a') { |f| "#{f.write(mail.encoded)}\r\n\r\n" }
       end
     end
-
   end
 end

lib/mail/network/delivery_methods/sendmail.rb

@@ -38,17 +38,15 @@ module Mail
   #
   #   mail.deliver!
   class Sendmail
-    include Mail::CheckDeliveryParams
+    attr_accessor :settings
 
     def initialize(values)
       self.settings = { :location       => '/usr/sbin/sendmail',
                         :arguments      => '-i' }.merge(values)
     end
 
-    attr_accessor :settings
-
     def deliver!(mail)
-      smtp_from, smtp_to, message = check_delivery_params(mail)
+      smtp_from, smtp_to, message = Mail::CheckDeliveryParams.check(mail)
 
       from = "-f #{self.class.shellquote(smtp_from)}"
       to = smtp_to.map { |_to| self.class.shellquote(_to) }.join(' ')

lib/mail/network/delivery_methods/smtp.rb

@@ -75,7 +75,7 @@ module Mail
   # 
   #   mail.deliver!
   class SMTP
-    include Mail::CheckDeliveryParams
+    attr_accessor :settings
 
     def initialize(values)
       self.settings = { :address              => "localhost",
@@ -91,12 +91,10 @@ def initialize(values)
                       }.merge!(values)
     end
 
-    attr_accessor :settings
-
     # Send the message via SMTP.
     # The from and to attributes are optional. If not set, they are retrieve from the Message.
     def deliver!(mail)
-      smtp_from, smtp_to, message = check_delivery_params(mail)
+      smtp_from, smtp_to, message = Mail::CheckDeliveryParams.check(mail)
 
       smtp = Net::SMTP.new(settings[:address], settings[:port])
       if settings[:tls] || settings[:ssl]
@@ -120,7 +118,6 @@ def deliver!(mail)
         self
       end
     end
-
 
     private
 

lib/mail/network/delivery_methods/smtp_connection.rb

@@ -38,25 +38,21 @@ module Mail
   # 
   #   mail.deliver!
   class SMTPConnection
-    include Mail::CheckDeliveryParams
+    attr_accessor :smtp, :settings
 
     def initialize(values)
       raise ArgumentError.new('A Net::SMTP object is required for this delivery method') if values[:connection].nil?
       self.smtp = values[:connection]
       self.settings = values
     end
 
-    attr_accessor :smtp
-    attr_accessor :settings
-
     # Send the message via SMTP.
     # The from and to attributes are optional. If not set, they are retrieve from the Message.
     def deliver!(mail)
-      smtp_from, smtp_to, message = check_delivery_params(mail)
+      smtp_from, smtp_to, message = Mail::CheckDeliveryParams.check(mail)
       response = smtp.sendmail(message, smtp_from, smtp_to)
 
       settings[:return_response] ? response : self
     end
-
   end
 end

lib/mail/network/delivery_methods/test_mailer.rb

@@ -8,10 +8,8 @@ module Mail
   # It also provides a template of the minimum methods you require to implement
   # if you want to make a custom mailer for Mail
   class TestMailer
-    include Mail::CheckDeliveryParams
-
     # Provides a store of all the emails sent with the TestMailer so you can check them.
-    def TestMailer.deliveries
+    def self.deliveries
       @@deliveries ||= []
     end
 
@@ -26,20 +24,19 @@ def TestMailer.deliveries
     # * length
     # * size
     # * and other common Array methods
-    def TestMailer.deliveries=(val)
+    def self.deliveries=(val)
       @@deliveries = val
     end
 
+    attr_accessor :settings
+
     def initialize(values)
       @settings = values.dup
     end
-
-    attr_accessor :settings
 
     def deliver!(mail)
-      check_delivery_params(mail)
+      Mail::CheckDeliveryParams.check(mail)
       Mail::TestMailer.deliveries << mail
     end
-
   end
 end

spec/mail/network/delivery_methods/exim_spec.rb

@@ -187,7 +187,7 @@
         subject "Email with no sender"
         body "body"
       end
-    end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+    end.to raise_error('SMTP From address may not be blank: nil')
   end
 
   it "should raise an error if no recipient if defined" do
@@ -200,6 +200,6 @@
         subject "Email with no recipient"
         body "body"
       end
-    end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+    end.to raise_error('SMTP To address may not be blank: []')
   end
 end

spec/mail/network/delivery_methods/file_delivery_spec.rb

@@ -101,7 +101,7 @@
           subject "Email with no sender"
           body "body"
         end
-      end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+      end.to raise_error('SMTP From address may not be blank: nil')
     end
 
     it "should raise an error if no recipient if defined" do
@@ -115,7 +115,7 @@
           subject "Email with no recipient"
           body "body"
         end
-      end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+      end.to raise_error('SMTP To address may not be blank: []')
     end
 
   end

spec/mail/network/delivery_methods/sendmail_spec.rb

@@ -206,7 +206,7 @@
         subject "Email with no sender"
         body "body"
       end
-    end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+    end.to raise_error('SMTP From address may not be blank: nil')
   end
 
   it "should raise an error if no recipient if defined" do
@@ -219,6 +219,6 @@
         subject "Email with no recipient"
         body "body"
       end
-    end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+    end.to raise_error('SMTP To address may not be blank: []')
   end
 end

spec/mail/network/delivery_methods/smtp_connection_spec.rb

@@ -60,7 +60,7 @@
         subject "Email with no sender"
         body "body"
       end
-    end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+    end.to raise_error('SMTP From address may not be blank: nil')
   end
 
   it "should raise an error if no recipient if defined" do
@@ -75,6 +75,6 @@
         subject "Email with no recipient"
         body "body"
       end
-    end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+    end.to raise_error('SMTP To address may not be blank: []')
   end
 end

spec/mail/network/delivery_methods/smtp_spec.rb

@@ -191,7 +191,7 @@ def redefine_verify_none(new_value)
           subject "Email with no sender"
           body "body"
         end
-      end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+      end.to raise_error('SMTP From address may not be blank: nil')
     end
 
     it "should raise an error if no recipient if defined" do
@@ -201,8 +201,63 @@ def redefine_verify_none(new_value)
           subject "Email with no recipient"
           body "body"
         end
-      end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+      end.to raise_error('SMTP To address may not be blank: []')
     end
-  end
 
+    it "should raise on SMTP injection via MAIL FROM newlines" do
+      addr = "[email protected]>\r\nDATA"
+
+      mail = Mail.new do
+        from addr
+        to "[email protected]"
+      end
+
+      # Mail 2.6.x header unfolding collapses whitespace, avoiding
+      # SMTP injection as a side effect.
+      expect(mail.smtp_envelope_from).to eq addr.gsub(/[\r\n]+/, ' ')
+    end
+
+    it "should raise on SMTP injection via RCPT TO newlines" do
+      addr = "[email protected]>\r\nDATA"
+
+      mail = Mail.new do
+        from "[email protected]"
+        to addr
+      end
+
+      # Mail 2.6.x header unfolding collapses whitespace, avoiding
+      # SMTP injection as a side effect.
+      expect(mail.smtp_envelope_to).to eq [addr.gsub(/[\r\n]+/, ' ')]
+    end
+
+    it "should raise on SMTP injection via MAIL FROM overflow" do
+      addr = "[email protected]#{'m' * 2025}DATA"
+
+      mail = Mail.new do
+        from addr
+        to "[email protected]"
+      end
+
+      expect(mail.smtp_envelope_from).to eq addr
+
+      expect do
+        mail.deliver
+      end.to raise_error(ArgumentError, "SMTP From address may not exceed 2kB: #{addr.inspect}")
+    end
+
+    it "should raise on SMTP injection via RCPT TO overflow" do
+      addr = "[email protected]#{'m' * 2027}DATA"
+
+      mail = Mail.new do
+        from "[email protected]"
+        to addr
+      end
+
+      expect(mail.smtp_envelope_to).to eq [addr]
+
+      expect do
+        mail.deliver
+      end.to raise_error(ArgumentError, "SMTP To address may not exceed 2kB: #{addr.inspect}")
+    end
+  end
 end

spec/mail/network/delivery_methods/test_mailer_spec.rb

@@ -65,7 +65,7 @@
         subject "Email with no sender"
         body "body"
       end
-    end.to raise_error('An SMTP From address is required to send a message. Set the message smtp_envelope_from, return_path, sender, or from address.')
+    end.to raise_error('SMTP From address may not be blank: nil')
   end
 
   it "should raise an error if no recipient if defined" do
@@ -78,7 +78,7 @@
         subject "Email with no recipient"
         body "body"
       end
-    end.to raise_error('An SMTP To address is required to send a message. Set the message smtp_envelope_to, to, cc, or bcc address.')
+    end.to raise_error('SMTP To address may not be blank: []')
   end
 
   it "should save settings passed to initialize" do