Fix fd leak and blind IO#gets in pwdump import

MSP-10715

Author: trevrosen

Gemfile

@@ -7,7 +7,7 @@ group :db do
   # Needed for Msf::DbManager
   gem 'activerecord', '>= 3.0.0', '< 4.0.0'
   # Metasploit::Credential database models
-  gem 'metasploit-credential', '>= 0.7.10.pre.core.pre.search', '< 0.8'
+  gem 'metasploit-credential', '~>0.8.2'
   # Database models shared between framework and Pro.
   gem 'metasploit_data_models', '~> 0.19'
   # Needed for module caching in Mdm::ModuleDetails

Gemfile.lock

@@ -41,7 +41,7 @@ GEM
       i18n (~> 0.6, >= 0.6.4)
       multi_json (~> 1.0)
     arel (3.0.3)
-    arel-helpers (2.0.0)
+    arel-helpers (2.0.1)
       activerecord (>= 3.1.0, < 5)
     bcrypt (3.1.7)
     builder (3.0.4)
@@ -60,21 +60,21 @@ GEM
     json (1.8.1)
     metasploit-concern (0.1.1)
       activesupport (~> 3.0, >= 3.0.0)
-    metasploit-credential (0.7.10.pre.core.pre.search)
+    metasploit-credential (0.8.2)
       metasploit-concern (~> 0.1.0)
-      metasploit-model (>= 0.25.6)
-      metasploit_data_models (~> 0.19)
+      metasploit-model (~> 0.26.1)
+      metasploit_data_models (~> 0.19.4)
       pg
       rubyntlm
       rubyzip (~> 1.1)
-    metasploit-model (0.25.6)
+    metasploit-model (0.26.1)
       activesupport
-    metasploit_data_models (0.19.0)
+    metasploit_data_models (0.19.4)
       activerecord (>= 3.2.13, < 4.0.0)
       activesupport
       arel-helpers
       metasploit-concern (~> 0.1.0)
-      metasploit-model (>= 0.25.1, < 0.26)
+      metasploit-model (~> 0.26.1)
       pg
     meterpreter_bins (0.0.6)
     method_source (0.8.2)
@@ -159,7 +159,7 @@ DEPENDENCIES
   factory_girl (>= 4.1.0)
   factory_girl_rails
   fivemat (= 1.2.1)
-  metasploit-credential (>= 0.7.10.pre.core.pre.search, < 0.8)
+  metasploit-credential (~> 0.8.2)
   metasploit-framework!
   metasploit_data_models (~> 0.19)
   network_interface (~> 0.0.1)

db/schema.rb

@@ -11,7 +11,7 @@
 #
 # It's strongly recommended to check this file into your version control system.
 
-ActiveRecord::Schema.define(:version => 20140703144541) do
+ActiveRecord::Schema.define(:version => 20140728191933) do
 
   create_table "api_keys", :force => true do |t|
     t.text     "token"
@@ -193,6 +193,9 @@
   add_index "metasploit_credential_cores", ["private_id"], :name => "index_metasploit_credential_cores_on_private_id"
   add_index "metasploit_credential_cores", ["public_id"], :name => "index_metasploit_credential_cores_on_public_id"
   add_index "metasploit_credential_cores", ["realm_id"], :name => "index_metasploit_credential_cores_on_realm_id"
+  add_index "metasploit_credential_cores", ["workspace_id", "private_id", "public_id"], :name => "unique_complete_metasploit_credential_cores", :unique => true
+  add_index "metasploit_credential_cores", ["workspace_id", "private_id"], :name => "unique_private_metasploit_credential_cores", :unique => true
+  add_index "metasploit_credential_cores", ["workspace_id", "public_id"], :name => "unique_public_metasploit_credential_cores", :unique => true
   add_index "metasploit_credential_cores", ["workspace_id"], :name => "index_metasploit_credential_cores_on_workspace_id"
 
   create_table "metasploit_credential_logins", :force => true do |t|

lib/msf/core/db.rb

@@ -2897,28 +2897,26 @@ def import_file(args={}, &block)
 
     data = ""
     ::File.open(filename, 'rb') do |f|
-      data = f.read(4)
+      # This check is the largest (byte-wise) that we need to do
+      # since the other 4-byte checks will be subsets of this larger one.
+      data = f.read(Metasploit::Credential::Exporter::Pwdump::FILE_ID_STRING.size)
     end
     if data.nil?
       raise DBImportError.new("Zero-length file")
     end
 
-    io = File.open(filename)
-    first_line = io.gets
-    io.rewind
-
-    if first_line.index("# Metasploit PWDump Export")
-      data = io
+    if data.index(Metasploit::Credential::Exporter::Pwdump::FILE_ID_STRING)
+      data = ::File.open(filename, 'rb')
     else
       case data[0,4]
-        when "PK\x03\x04"
-          data = Zip::File.open(filename)
-        when "\xd4\xc3\xb2\xa1", "\xa1\xb2\xc3\xd4"
-          data = PacketFu::PcapFile.new(:filename => filename)
-        else
-          ::File.open(filename, 'rb') do |f|
-            sz = f.stat.size
-            data = f.read(sz)
+      when "PK\x03\x04"
+        data = Zip::File.open(filename)
+      when "\xd4\xc3\xb2\xa1", "\xa1\xb2\xc3\xd4"
+        data = PacketFu::PcapFile.new(:filename => filename)
+      else
+        ::File.open(filename, 'rb') do |f|
+          sz = f.stat.size
+          data = f.read(sz)
         end
       end
     end
@@ -2929,7 +2927,6 @@ def import_file(args={}, &block)
     else
       import(args.merge(:data => data))
     end
-
   end
 
   # A dispatcher method that figures out the data's file type,
@@ -3539,6 +3536,7 @@ def import_msf_pwdump(args={}, &block)
     origin   = Metasploit::Credential::Origin::Import.create!(filename: filename)
     importer = Metasploit::Credential::Importer::Pwdump.new(input: args[:data], workspace: wspace, filename: filename, origin:origin)
     importer.import!
+    importer.input.close unless importer.input.closed?
   end
 
   # If hex notation is present, turn them into a character.