Skip to content

Commit dbe9b47

Browse files
committed
lands 3469, fixes handler deadlock in corner cases
May affect the following RM issues which need to be retested: https://dev.metasploit.com/redmine/issues/8407 https://dev.metasploit.com/redmine/issues/4314 https://dev.metasploit.com/redmine/issues/6829
2 parents 74c1bfe + d6a263d commit dbe9b47

File tree

3 files changed

+25
-15
lines changed

3 files changed

+25
-15
lines changed

lib/msf/core/handler/reverse_tcp.rb

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -55,11 +55,12 @@ def initialize(info = {})
5555
OptAddress.new('ReverseListenerBindAddress', [ false, 'The specific IP address to bind to on the local system']),
5656
OptInt.new('ReverseListenerBindPort', [ false, 'The port to bind to on the local system if different from LPORT' ]),
5757
OptString.new('ReverseListenerComm', [ false, 'The specific communication channel to use for this listener']),
58-
OptBool.new('ReverseAllowProxy', [ true, 'Allow reverse tcp even with Proxies specified. Connect back will NOT go through proxy but directly to LHOST', false])
58+
OptBool.new('ReverseAllowProxy', [ true, 'Allow reverse tcp even with Proxies specified. Connect back will NOT go through proxy but directly to LHOST', false]),
59+
OptBool.new('ReverseListenerThreaded', [ true, 'Handle every connection in a new thread (experimental)', false])
5960
], Msf::Handler::ReverseTcp)
6061

61-
6262
self.handler_queue = ::Queue.new
63+
self.conn_threads = []
6364
end
6465

6566
#
@@ -124,6 +125,12 @@ def setup_handler
124125
#
125126
def cleanup_handler
126127
stop_handler
128+
129+
# Kill any remaining handle_connection threads that might
130+
# be hanging around
131+
conn_threads.each { |thr|
132+
thr.kill rescue nil
133+
}
127134
end
128135

129136
#
@@ -154,7 +161,13 @@ def start_handler
154161
while true
155162
client = self.handler_queue.pop
156163
begin
157-
handle_connection(wrap_aes_socket(client))
164+
if datastore['ReverseListenerThreaded']
165+
self.conn_threads << framework.threads.spawn("ReverseTcpHandlerSession-#{local_port}-#{client.peerhost}", false, client) { | client_copy|
166+
handle_connection(wrap_aes_socket(client_copy))
167+
}
168+
else
169+
handle_connection(wrap_aes_socket(client))
170+
end
158171
rescue ::Exception
159172
elog("Exception raised from handle_connection: #{$!.class}: #{$!}\n\n#{$@.join("\n")}")
160173
end
@@ -261,6 +274,7 @@ def bind_address
261274
attr_accessor :listener_thread # :nodoc:
262275
attr_accessor :handler_thread # :nodoc:
263276
attr_accessor :handler_queue # :nodoc:
277+
attr_accessor :conn_threads # :nodoc:
264278
end
265279

266280
end

lib/msf/core/handler/reverse_tcp_double.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,7 @@ def cleanup_handler
8383
# Kill any remaining handle_connection threads that might
8484
# be hanging around
8585
conn_threads.each { |thr|
86-
thr.kill
86+
thr.kill rescue nil
8787
}
8888
end
8989

@@ -105,9 +105,6 @@ def start_handler
105105

106106
client_b = self.listener_sock.accept
107107
print_status("Accepted the second client connection...")
108-
109-
sock_inp, sock_out = detect_input_output(client_a, client_b)
110-
111108
rescue
112109
wlog("Exception raised during listener accept: #{$!}\n\n#{$@.join("\n")}")
113110
return nil
@@ -119,9 +116,10 @@ def start_handler
119116
# Start a new thread and pass the client connection
120117
# as the input and output pipe. Client's are expected
121118
# to implement the Stream interface.
122-
conn_threads << framework.threads.spawn("ReverseTcpDoubleHandlerSession", false, sock_inp, sock_out) { | sock_inp_copy, sock_out_copy|
119+
conn_threads << framework.threads.spawn("ReverseTcpDoubleHandlerSession", false, client_a, client_b) { | client_a_copy, client_b_copy|
123120
begin
124-
chan = TcpReverseDoubleSessionChannel.new(framework, sock_inp_copy, sock_out_copy)
121+
sock_inp, sock_out = detect_input_output(client_a_copy, client_b_copy)
122+
chan = TcpReverseDoubleSessionChannel.new(framework, sock_inp, sock_out)
125123
handle_connection(chan.lsock)
126124
rescue
127125
elog("Exception raised from handle_connection: #{$!}\n\n#{$@.join("\n")}")

lib/msf/core/handler/reverse_tcp_double_ssl.rb

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def cleanup_handler
8484
# Kill any remaining handle_connection threads that might
8585
# be hanging around
8686
conn_threads.each { |thr|
87-
thr.kill
87+
thr.kill rescue nil
8888
}
8989
end
9090

@@ -106,9 +106,6 @@ def start_handler
106106

107107
client_b = self.listener_sock.accept
108108
print_status("Accepted the second client connection...")
109-
110-
sock_inp, sock_out = detect_input_output(client_a, client_b)
111-
112109
rescue
113110
wlog("Exception raised during listener accept: #{$!}\n\n#{$@.join("\n")}")
114111
return nil
@@ -120,9 +117,10 @@ def start_handler
120117
# Start a new thread and pass the client connection
121118
# as the input and output pipe. Client's are expected
122119
# to implement the Stream interface.
123-
conn_threads << framework.threads.spawn("ReverseTcpDoubleSSLHandlerSession", false, sock_inp, sock_out) { | sock_inp_copy, sock_out_copy|
120+
conn_threads << framework.threads.spawn("ReverseTcpDoubleSSLHandlerSession", false, client_a, client_b) { | client_a_copy, client_b_copy|
124121
begin
125-
chan = TcpReverseDoubleSSLSessionChannel.new(framework, sock_inp_copy, sock_out_copy)
122+
sock_inp, sock_out = detect_input_output(client_a_copy, client_b_copy)
123+
chan = TcpReverseDoubleSSLSessionChannel.new(framework, sock_inp, sock_out)
126124
handle_connection(chan.lsock)
127125
rescue
128126
elog("Exception raised from handle_connection: #{$!}\n\n#{$@.join("\n")}")

0 commit comments

Comments
 (0)