From 6fcc8649427144b97ffce004c8f68059c6308c75 Mon Sep 17 00:00:00 2001 From: HD Moore Date: Tue, 2 Sep 2014 13:29:37 -0500 Subject: [PATCH 1/4] Reduce the chance of file descriptor leaks in SMBServer This patch addresses three observed error conditions in long-running SMB services. 1. A call to get_once() in on_client_data could raise a Timeout exception and bubble all the way up to the dispatcher. This should technically never happen, but gets triggered for zero-byte writes and clients closing their connections. The fix was to handle the exception and lower the timeout. The change was tested with a number of SMB clients to make sure this didn't introduce any regressions. 2. A client could indefinitely keep a connection to the SMB server. The SMB server now disconnects idle clients after 120 seconds of inactivity (configurable). 3. A client could send a large amount of data that was invalid SMB traffic, using up memory as a potential DoS. Caveats: The idle client sweep occurs every 100 requests or at an interval equal to the idle timeout. A client could fill up the entire connection table on its own, preventing the sweep from occurring by preventing new connections. Fixing this would require a dedicated thread to sweep for idle connections and is a more aggressive attack than this patch is designed to defend against (accidental connection flooding, basically). --- lib/msf/core/exploit/smb.rb | 64 ++++++++++++++++++++++++++++++++++++- 1 file changed, 63 insertions(+), 1 deletion(-) diff --git a/lib/msf/core/exploit/smb.rb b/lib/msf/core/exploit/smb.rb index 095adf45bc..a26dc66663 100644 --- a/lib/msf/core/exploit/smb.rb +++ b/lib/msf/core/exploit/smb.rb @@ -698,6 +698,16 @@ module Exploit::Remote::SMBServer [ OptPort.new('SRVPORT', [ true, "The local port to listen on.", 445 ]) ], self.class) + + register_advanced_options( + [ + OptInt.new('SMBServerMaximumBuffer', [ true, "The maximum number of data in megabytes to buffer", 2 ]), + OptInt.new('SMBServerIdleTimeout', [ true, "The maximum amount of time to keep an idle session open in seconds", 120 ]), + ], self.class) + + @smb_server_last_pool_sweep = Time.now.to_f + @smb_server_pool_mutex = Mutex.new + @smb_server_request_counter = 0 end def setup @@ -722,16 +732,40 @@ module Exploit::Remote::SMBServer def smb_conn(c) @state[c] = {:name => "#{c.peerhost}:#{c.peerport}", :ip => c.peerhost, :port => c.peerport} + smb_pool_update(c) end def smb_stop(c) + # Make sure the socket is closed + c.close rescue nil + # Delete the state table entry @state.delete(c) end def smb_recv(c) smb = @state[c] smb[:data] ||= '' - smb[:data] << c.get_once + + # Capture any low-level timeout exceptions to prevent it from bubbling + buff = c.get_once(-1, 0.25) rescue nil + + # The client said it had data, but lied, kill the session + unless buff and buff.length > 0 + smb_stop(c) + return + end + + # Append the new data to the buffer + smb[:data] << buff + + # Prevent a simplistic DoS if the buffer is too big + if smb[:data].length > (1024*1024*datastore['SMBServerMaximumBuffer']) + smb_stop(c) + return + end + + # Update the last-seen timestamp and purge old entries + smb_pool_update(c) while(smb[:data].length > 0) @@ -821,6 +855,34 @@ module Exploit::Remote::SMBServer c.put(pkt.to_s) end + # Update the last-seen timestamp and purge old entries + def smb_pool_update(c) + + @state[c][:last_action] = Time.now.to_f + @smb_server_request_counter += 1 + + unless @smb_server_request_counter % 100 == 0 || + @smb_server_last_pool_sweep + smb_idle_timeout < Time.now.to_f + return + end + + # Synchronize pool sweeps in case we move to threaded services + @smb_server_pool_mutex.synchronize do + purge_list = [] + + @smb_server_last_pool_sweep = Time.now.to_f + + @state.keys.each do |sc| + if @state[sc][:last_action] + datastore['SMBServerIdleTimeout'].to_f < Time.now.to_f + purge_list << sc + end + end + + # Purge any idle connections to rescue file descriptors + purge_list.each { |sc| smb_stop(sc) } + end + end + end From 85c5de07ecfbb54cd77d63f1a931e808c85e0f62 Mon Sep 17 00:00:00 2001 From: HD Moore Date: Tue, 2 Sep 2014 13:47:01 -0500 Subject: [PATCH 2/4] Fix use of datastore['SMBServerIdleTimeout'] --- lib/msf/core/exploit/smb.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/msf/core/exploit/smb.rb b/lib/msf/core/exploit/smb.rb index a26dc66663..7cd132bb5c 100644 --- a/lib/msf/core/exploit/smb.rb +++ b/lib/msf/core/exploit/smb.rb @@ -862,7 +862,7 @@ module Exploit::Remote::SMBServer @smb_server_request_counter += 1 unless @smb_server_request_counter % 100 == 0 || - @smb_server_last_pool_sweep + smb_idle_timeout < Time.now.to_f + @smb_server_last_pool_sweep + datastore['SMBServerIdleTimeout'].to_f < Time.now.to_f return end From 4966082de5105a0f8315c393c266f063e671406e Mon Sep 17 00:00:00 2001 From: HD Moore Date: Wed, 3 Sep 2014 23:06:11 -0500 Subject: [PATCH 3/4] Replace 'rescue nil' with DRY-violating versions :( --- lib/msf/core/exploit/smb.rb | 22 +++++++++++++++++++--- 1 file changed, 19 insertions(+), 3 deletions(-) diff --git a/lib/msf/core/exploit/smb.rb b/lib/msf/core/exploit/smb.rb index 7cd132bb5c..85b1bfc07f 100644 --- a/lib/msf/core/exploit/smb.rb +++ b/lib/msf/core/exploit/smb.rb @@ -736,8 +736,16 @@ module Exploit::Remote::SMBServer end def smb_stop(c) + # Make sure the socket is closed - c.close rescue nil + begin + c.close + # Handle any number of errors that a double-close or failed shutdown can trigger + rescue ::IOError, ::EOFError, + ::Errno::ECONNRESET, ::Errno::ENOTCONN, ::Errno::ECONNABORTED, + ::Errno::ETIMEDOUT, ::Errno::ENETRESET, ::Errno::ESHUTDOWN + end + # Delete the state table entry @state.delete(c) end @@ -746,8 +754,16 @@ module Exploit::Remote::SMBServer smb = @state[c] smb[:data] ||= '' - # Capture any low-level timeout exceptions to prevent it from bubbling - buff = c.get_once(-1, 0.25) rescue nil + buff = '' + begin + buff = c.get_once(-1, 0.25) + # Handle any number of errors that a read can trigger depending on socket state + rescue ::IOError, ::EOFError, + ::Errno::ECONNRESET, ::Errno::ENOTCONN, ::Errno::ECONNABORTED, + ::Errno::ETIMEDOUT, ::Errno::ENETRESET, ::Errno::ESHUTDOWN + smb_stop(c) + return + end # The client said it had data, but lied, kill the session unless buff and buff.length > 0 From 8fa666b75d47ead50be208c9b05034fd09725d7f Mon Sep 17 00:00:00 2001 From: HD Moore Date: Sun, 28 Sep 2014 17:41:21 -0700 Subject: [PATCH 4/4] Verbose messages on why a connection is closed --- lib/msf/core/exploit/smb.rb | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/lib/msf/core/exploit/smb.rb b/lib/msf/core/exploit/smb.rb index 85b1bfc07f..587d716c98 100644 --- a/lib/msf/core/exploit/smb.rb +++ b/lib/msf/core/exploit/smb.rb @@ -760,13 +760,15 @@ module Exploit::Remote::SMBServer # Handle any number of errors that a read can trigger depending on socket state rescue ::IOError, ::EOFError, ::Errno::ECONNRESET, ::Errno::ENOTCONN, ::Errno::ECONNABORTED, - ::Errno::ETIMEDOUT, ::Errno::ENETRESET, ::Errno::ESHUTDOWN + ::Errno::ETIMEDOUT, ::Errno::ENETRESET, ::Errno::ESHUTDOWN + vprint_status("Dropping connection from #{smb[:name]} due to exception: #{$!.class} #{$!}") smb_stop(c) return end # The client said it had data, but lied, kill the session unless buff and buff.length > 0 + vprint_status("Dropping connection from #{smb[:name]} due to empty payload...") smb_stop(c) return end @@ -776,6 +778,7 @@ module Exploit::Remote::SMBServer # Prevent a simplistic DoS if the buffer is too big if smb[:data].length > (1024*1024*datastore['SMBServerMaximumBuffer']) + vprint_status("Dropping connection from #{smb[:name]} due to oversized buffer of #{smb[:data].length} bytes...") smb_stop(c) return end @@ -823,10 +826,11 @@ module Exploit::Remote::SMBServer pkt = CONST::SMB_BASE_PKT.make_struct pkt.from_s(buff) - # Only response to requests, ignore server replies + # Only respond to requests, ignore server replies if (pkt['Payload']['SMB'].v['Flags1'] & 128 != 0) - print_status("Ignoring server response from #{smb[:name]}") - next + vprint_status("Dropping connection from #{smb[:name]} due to missing client request flag") + smb_stop(c) + return end cmd = pkt['Payload']['SMB'].v['Command'] @@ -895,7 +899,10 @@ module Exploit::Remote::SMBServer end # Purge any idle connections to rescue file descriptors - purge_list.each { |sc| smb_stop(sc) } + purge_list.each do |sc| + vprint_status("Dropping connection from #{@state[sc][:name]} due to idle timeout...") + smb_stop(sc) + end end end