Skip to content

Commit

Permalink
Fix security vulnerability allowing command line exploit when using e…
Browse files Browse the repository at this point in the history
…xim or sendmail from the command line
  • Loading branch information
Mikel Lindsaar committed Mar 6, 2012
1 parent 47e288e commit ac56f03
Show file tree
Hide file tree
Showing 7 changed files with 277 additions and 67 deletions.
2 changes: 1 addition & 1 deletion lib/mail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ module Mail # :doc:
require 'mail/core_extensions/nil'
require 'mail/core_extensions/object'
require 'mail/core_extensions/string'
require 'mail/core_extensions/shellwords' unless String.new.respond_to?(:shellescape)
require 'mail/core_extensions/shell_escape'
require 'mail/core_extensions/smtp' if RUBY_VERSION < '1.9.3'
require 'mail/indifferent_hash'

Expand Down
56 changes: 56 additions & 0 deletions lib/mail/core_extensions/shell_escape.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
# encoding: utf-8

# The following is an adaptation of ruby 1.9.2's shellwords.rb file,
# it is modified to include '+' in the allowed list to allow for
# sendmail to accept email addresses as the sender with a + in them
#
module Mail
module ShellEscape
# Escapes a string so that it can be safely used in a Bourne shell
# command line.
#
# Note that a resulted string should be used unquoted and is not
# intended for use in double quotes nor in single quotes.
#
# open("| grep #{Shellwords.escape(pattern)} file") { |pipe|
# # ...
# }
#
# +String#shellescape+ is a shorthand for this function.
#
# open("| grep #{pattern.shellescape} file") { |pipe|
# # ...
# }
#
def escape_for_shell(str)
# An empty argument will be skipped, so return empty quotes.
return "''" if str.empty?

str = str.dup

# Process as a single byte sequence because not all shell
# implementations are multibyte aware.
str.gsub!(/([^A-Za-z0-9_\s\+\-.,:\/@\n])/n, "\\\\\\1")

# A LF cannot be escaped with a backslash because a backslash + LF
# combo is regarded as line continuation and simply ignored.
str.gsub!(/\n/, "'\n'")

return str
end

module_function :escape_for_shell
end
end

class String
# call-seq:
# str.shellescape => string
#
# Escapes +str+ so that it can be safely used in a Bourne shell
# command line. See +Shellwords::shellescape+ for details.
#
def escape_for_shell
Mail::ShellEscape.escape_for_shell(self)
end
end
57 changes: 0 additions & 57 deletions lib/mail/core_extensions/shellwords.rb

This file was deleted.

43 changes: 38 additions & 5 deletions lib/mail/network/delivery_methods/exim.rb
Original file line number Diff line number Diff line change
@@ -1,12 +1,45 @@
module Mail

# A delivery method implementation which sends via exim.
#
# To use this, first find out where the exim binary is on your computer,
# if you are on a mac or unix box, it is usually in /usr/sbin/exim, this will
# be your exim location.
#
# Mail.defaults do
# delivery_method :exim
# end
#
# Or if your exim binary is not at '/usr/sbin/exim'
#
# Mail.defaults do
# delivery_method :exim, :location => '/absolute/path/to/your/exim'
# end
#
# Then just deliver the email as normal:
#
# Mail.deliver do
# to '[email protected]'
# from '[email protected]'
# subject 'testing exim'
# body 'testing exim'
# end
#
# Or by calling deliver on a Mail message
#
# mail = Mail.new do
# to '[email protected]'
# from '[email protected]'
# subject 'testing exim'
# body 'testing exim'
# end
#
# mail.deliver!
class Exim < Sendmail

def deliver!(mail)
envelope_from = mail.return_path || mail.sender || mail.from_addrs.first
return_path = "-f \"#{envelope_from.to_s.shellescape}\"" if envelope_from
arguments = [settings[:arguments], return_path].compact.join(" ")
self.class.call(settings[:location], arguments, mail)
def initialize(values)
self.settings = { :location => '/usr/sbin/exim',
:arguments => '-i -t' }.merge(values)
end

def self.call(path, arguments, mail)
Expand Down
6 changes: 3 additions & 3 deletions lib/mail/network/delivery_methods/sendmail.rb
Original file line number Diff line number Diff line change
Expand Up @@ -45,14 +45,14 @@ def initialize(values)

def deliver!(mail)
envelope_from = mail.return_path || mail.sender || mail.from_addrs.first
return_path = "-f \"#{envelope_from.to_s.gsub('"', '\"')}\"" if envelope_from
return_path = "-f " + '"' + envelope_from.escape_for_shell + '"' if envelope_from

arguments = [settings[:arguments], return_path].compact.join(" ")

Sendmail.call(settings[:location], arguments, mail.destinations.collect(&:shellescape).join(" "), mail)
self.class.call(settings[:location], arguments, mail.destinations.collect(&:shellescape).join(" "), mail)
end

def Sendmail.call(path, arguments, destinations, mail)
def self.call(path, arguments, destinations, mail)
IO.popen("#{path} #{arguments} #{destinations}", "w+") do |io|
io.puts mail.encoded.to_lf
io.flush
Expand Down
161 changes: 161 additions & 0 deletions spec/mail/network/delivery_methods/exim_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,161 @@
# encoding: utf-8
require 'spec_helper'

describe "exim delivery agent" do

before(:each) do
# Reset all defaults back to original state
Mail.defaults do
delivery_method :smtp, { :address => "localhost",
:port => 25,
:domain => 'localhost.localdomain',
:user_name => nil,
:password => nil,
:authentication => nil,
:enable_starttls_auto => true }
end
end

it "should send an email using exim" do
Mail.defaults do
delivery_method :exim
end

mail = Mail.new do
from '[email protected]'
to '[email protected], [email protected]'
subject 'invalid RFC2822'
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-i -t -f "[email protected]"',
'[email protected] [email protected]',
mail)
mail.deliver!
end

describe "return path" do

it "should send an email with a return-path using exim" do
Mail.defaults do
delivery_method :exim
end

mail = Mail.new do
to "[email protected]"
from "[email protected]"
sender "[email protected]"
subject "Can't set the return-path"
return_path "[email protected]"
message_id "<[email protected]>"
body "body"
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-i -t -f "[email protected]"',
'[email protected]',
mail)

mail.deliver

end

it "should use the sender address is no return path is specified" do
Mail.defaults do
delivery_method :exim
end

mail = Mail.new do
to "[email protected]"
from "[email protected]"
sender "[email protected]"
subject "Can't set the return-path"
message_id "<[email protected]>"
body "body"
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-i -t -f "[email protected]"',
'[email protected]',
mail)

mail.deliver
end

it "should use the from address is no return path or sender are specified" do
Mail.defaults do
delivery_method :exim
end

mail = Mail.new do
to "[email protected]"
from "[email protected]"
subject "Can't set the return-path"
message_id "<[email protected]>"
body "body"
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-i -t -f "[email protected]"',
'[email protected]',
mail)
mail.deliver
end

it "should escape the return path address" do
Mail.defaults do
delivery_method :exim
end

mail = Mail.new do
to '[email protected]'
from '"from+suffix test"@test.lindsaar.net'
subject 'Can\'t set the return-path'
message_id '<[email protected]>'
body 'body'
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-i -t -f "\"from+suffix test\"@test.lindsaar.net"',
'[email protected]',
mail)
mail.deliver
end
end

it "should still send an email if the settings have been set to nil" do
Mail.defaults do
delivery_method :exim, :arguments => nil
end

mail = Mail.new do
from '[email protected]'
to '[email protected], [email protected]'
subject 'invalid RFC2822'
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
'-f "[email protected]"',
'[email protected] [email protected]',
mail)
mail.deliver!
end

it "should escape evil haxxor attemptes" do
Mail.defaults do
delivery_method :exim, :arguments => nil
end

mail = Mail.new do
from '"foo\";touch /tmp/PWNED;\""@blah.com'
to '[email protected]'
subject 'invalid RFC2822'
end

Mail::Exim.should_receive(:call).with('/usr/sbin/exim',
"-f \"\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com\"",
'[email protected]',
mail)
mail.deliver!
end
end
19 changes: 18 additions & 1 deletion spec/mail/network/delivery_methods/sendmail_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@
end
end


it "should still send an email if the settings have been set to nil" do
Mail.defaults do
delivery_method :sendmail, :arguments => nil
Expand All @@ -141,4 +140,22 @@
mail)
mail.deliver!
end

it "should escape evil haxxor attemptes" do
Mail.defaults do
delivery_method :sendmail, :arguments => nil
end

mail = Mail.new do
from '"foo\";touch /tmp/PWNED;\""@blah.com'
to '[email protected]'
subject 'invalid RFC2822'
end

Mail::Sendmail.should_receive(:call).with('/usr/sbin/sendmail',
"-f \"\\\"foo\\\\\\\"\\;touch /tmp/PWNED\\;\\\\\\\"\\\"@blah.com\"",
'[email protected]',
mail)
mail.deliver!
end
end

0 comments on commit ac56f03

Please sign in to comment.