CVE-2012-2139: Improper Limitation of a Pathname to a Restricted Directory ('Path Traversal') (CWE-22)
value: Directory traversal vulnerability in lib/mail/network/delivery_methods/file_delivery.rb in the Mail gem before 2.4.4 for Ruby allows remote attackers to read arbitrary files via a .. (dot dot) in the to parameter.
CWE description: The product uses external input to construct a pathname that is intended to identify a file or directory that is located underneath a restricted parent directory, but the product does not properly neutralize special elements within the pathname that can cause the pathname to resolve to a location that is outside of the restricted directory.
Published: 2012-07-18T18:55Z
Repository: https://github.com/mikel/mail
Language: Ruby
Files changed: lib/mail/network/delivery_methods/file_delivery.rb, spec/mail/network/delivery_methods/file_delivery_spec.rb
Commit date: 2012-03-14 22:44:27 +1100
Commit message: Preventing file system traversal in file_delivery method

Diff: diff --git a/lib/mail/network/delivery_methods/file_delivery.rb b/lib/mail/network/delivery_methods/file_delivery.rb
index fc3e3b2..fec750a 100644
--- a/lib/mail/network/delivery_methods/file_delivery.rb
+++ b/lib/mail/network/delivery_methods/file_delivery.rb
@@ -4,11 +4,11 @@ module Mail
   # address.  Each file is appended to if it already exists.
   # 
   # So if you have an email going to fred@test, bob@test, joe@anothertest, and you
   # set your location path to /path/to/mails then FileDelivery will create the directory
   # if it does not exist, and put one copy of the email in three files, called
-  # "fred@test", "bob@test" and "joe@anothertest"
+  # by their message id
   # 
   # Make sure the path you specify with :location is writable by the Ruby process
   # running Mail.
   class FileDelivery
 
@@ -30,11 +30,11 @@ module Mail
       else
         ::FileUtils.mkdir_p settings[:location]
       end
 
       mail.destinations.uniq.each do |to|
-        ::File.open(::File.join(settings[:location], to), 'a') { |f| "#{f.write(mail.encoded)}\r\n\r\n" }
+        ::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
diff --git a/spec/mail/network/delivery_methods/file_delivery_spec.rb b/spec/mail/network/delivery_methods/file_delivery_spec.rb
index 10d16eb..5aed800 100644
--- a/spec/mail/network/delivery_methods/file_delivery_spec.rb
+++ b/spec/mail/network/delivery_methods/file_delivery_spec.rb
@@ -72,8 +72,23 @@ describe "SMTP Delivery Method" do
       end
       delivery = File.join(Mail.delivery_method.settings[:location], 'mikel@test.lindsaar.net')
       File.exists?(delivery).should be_true
     end
 
+    it "should use the base name of the file name to prevent file system traversal" do
+      Mail.defaults do
+        delivery_method :file, :location => tmpdir
+      end
+      
+      Mail.deliver do
+        from    'roger@moore.com'
+        to      '../../../../../../../../../../../tmp/pwn'
+        subject 'evil hacker'
+      end
+
+      delivery = File.join(Mail.delivery_method.settings[:location], 'pwn')
+      File.exists?(delivery).should be_true
+    end
+
   end
   
 end

Vulnerable code:   # "fred@test", "bob@test" and "joe@anothertest"
        ::File.open(::File.join(settings[:location], to), 'a') { |f| "#{f.write(mail.encoded)}\r\n\r\n" }
Fixed code:   # by their message id
        ::File.open(::File.join(settings[:location], File.basename(to.to_s)), 'a') { |f| "#{f.write(mail.encoded)}\r\n\r\n" }
    it "should use the base name of the file name to prevent file system traversal" do
      Mail.defaults do
        delivery_method :file, :location => tmpdir
      end
      
      Mail.deliver do
        from    'roger@moore.com'
        to      '../../../../../../../../../../../tmp/pwn'
        subject 'evil hacker'
      end

      delivery = File.join(Mail.delivery_method.settings[:location], 'pwn')
      File.exists?(delivery).should be_true
    end