From dd57138423f3292d40bffd43a0daad92412c67d1 Mon Sep 17 00:00:00 2001 From: Adam Cammack Date: Mon, 20 Nov 2017 16:52:05 -0600 Subject: [PATCH] Make external module read loop more robust Changes from a "hope we get at most one message at a time" model to something beginning to resemble a state machine. Also logs error output and fails the MSF module when the external module fails. --- lib/msf/core/module/external.rb | 27 +++++----- lib/msf/core/modules/external/bridge.rb | 66 +++++++++++++++++-------- modules/exploits/linux/smtp/haraka.py | 1 + 3 files changed, 62 insertions(+), 32 deletions(-) diff --git a/lib/msf/core/module/external.rb b/lib/msf/core/module/external.rb index ed9705a942..5a6c1bf29d 100644 --- a/lib/msf/core/module/external.rb +++ b/lib/msf/core/module/external.rb @@ -2,19 +2,24 @@ include Msf::Auxiliary::Report module Msf::Module::External def wait_status(mod) - while mod.running - m = mod.get_status - if m - case m.method - when :message - log_output(m) - when :report - process_report(m) - when :reply - # we're done - break + begin + while mod.running + m = mod.get_status + if m + case m.method + when :message + log_output(m) + when :report + process_report(m) + when :reply + # we're done + break + end end end + rescue Exception => e #Msf::Modules::External::Bridge::Error => e + elog e.backtrace.join("\n") + fail_with Failure::UNKNOWN, e.message end end diff --git a/lib/msf/core/modules/external/bridge.rb b/lib/msf/core/modules/external/bridge.rb index 5ffcc6b0a2..c150ae258e 100644 --- a/lib/msf/core/modules/external/bridge.rb +++ b/lib/msf/core/modules/external/bridge.rb @@ -43,12 +43,13 @@ class Msf::Modules::External::Bridge self.path = module_path self.cmd = [self.path, self.path] self.messages = Queue.new + self.buf = '' end protected attr_writer :path, :running - attr_accessor :cmd, :env, :ios, :messages + attr_accessor :cmd, :env, :ios, :buf, :messages, :wait_thread def describe resp = send_receive(Msf::Modules::External::Message.new(:describe)) @@ -64,8 +65,9 @@ class Msf::Modules::External::Bridge end def send(message) - input, output, status = ::Open3.popen3(self.env, self.cmd) - self.ios = [input, output, status] + input, output, err, status = ::Open3.popen3(self.env, self.cmd) + self.ios = [input, output, err] + self.wait_thread = status # We would call Rex::Threadsafe directly, but that would require rex for standalone use case select(nil, [input], nil, 0.1) when nil @@ -91,33 +93,55 @@ class Msf::Modules::External::Bridge end def recv(filter_id=nil, timeout=600) - _, fd, _ = self.ios + _, out, err = self.ios + message = '' # Multiple messages can come over the wire all at once, and since yajl # doesn't play nice with windows, we have to emulate a state machine to # read just enough off the wire to get one request at a time. Since # Windows cannot do a nonblocking read on a pipe, we are forced to do a - # whole lot of `select` syscalls :( - buf = "" + # whole lot of `select` syscalls and keep a buffer ourselves :( begin loop do + # This is so we don't end up calling JSON.parse on every char and + # catch an exception. Windows can't do nonblock on pipes, so we + # still have to do the select if we are not at the end of object + # and don't have any buffer left + parts = self.buf.split '}', 2 + if parts.length == 2 # [part, rest] + message << parts[0] << '}' + self.buf = parts[1] + break + elsif parts.length == 1 # [part] + if self.buf[-1] == '}' + message << parts[0] << '}' + self.buf = '' + break + else + message << parts[0] + self.buf = '' + end + end + # We would call Rex::Threadsafe directly, but that would require Rex for standalone use - case select([fd], nil, nil, timeout) - when nil + res = select([out, err], nil, nil, timeout) + if res == nil # This is what we would have gotten without Rex and what `readpartial` can also raise raise EOFError.new - when [[fd], [], []] - c = fd.readpartial(1) - buf << c - - # This is so we don't end up calling JSON.parse on every char and - # having to catch an exception. Windows can't do nonblock on pipes, - # so we still have to do the select each time. - break if c == '}' + else + fds = res[0] + # Preferentially drain and log stderr + if fds.include? err + errbuf = err.readpartial(4096) + elog "Unexpected output running #{self.path}:\n#{errbuf}" + end + if fds.include? out + self.buf << out.readpartial(4096) + end end end - m = Msf::Modules::External::Message.from_module(JSON.parse(buf)) + m = Msf::Modules::External::Message.from_module(JSON.parse(message)) if filter_id && m.id != filter_id # We are filtering for a response to a particular message, but we got # something else, store the message and try again @@ -128,16 +152,16 @@ class Msf::Modules::External::Bridge m end rescue JSON::ParserError - # Probably an incomplete response, but no way to really tell + # Probably an incomplete response, but no way to really tell. Keep trying + # until EOF retry rescue EOFError => e - {} + nil end end def close_ios - input, output, status = self.ios - [input, output].each {|fd| fd.close rescue nil} # Yeah, yeah. I know. + self.ios.each {|fd| fd.close rescue nil} # Yeah, yeah. I know. end end diff --git a/modules/exploits/linux/smtp/haraka.py b/modules/exploits/linux/smtp/haraka.py index 7033d90ef2..59dff0bb03 100755 --- a/modules/exploits/linux/smtp/haraka.py +++ b/modules/exploits/linux/smtp/haraka.py @@ -72,6 +72,7 @@ def send_mail(to, mailserver, cmd, mfrom, port): except smtplib.SMTPDataError as err: if err[0] == 450: module.log("Triggered bug in target server (%s)"%err[1], 'good') + s.close() return(True) module.log("Bug not triggered in target server", 'error') module.log("it may not be vulnerable or have the attachment plugin activated", 'error')