From 2067d7267ec2d65b990e0a4e60298afc27376854 Mon Sep 17 00:00:00 2001
From: Spencer McIntyre <zeroSteiner@gmail.com>
Date: Thu, 26 Aug 2021 18:44:17 -0400
Subject: [PATCH] Cleanup the process object

When a child process exits, the corresponding object should be closed
and cleanup all of it's streams and threads as necessary.
---
 python/meterpreter/ext_server_stdapi.py |  1 +
 python/meterpreter/meterpreter.py       | 54 +++++++++++++++----------
 2 files changed, 34 insertions(+), 21 deletions(-)

diff --git a/python/meterpreter/ext_server_stdapi.py b/python/meterpreter/ext_server_stdapi.py
index 1f332288..45692a51 100644
--- a/python/meterpreter/ext_server_stdapi.py
+++ b/python/meterpreter/ext_server_stdapi.py
@@ -1167,6 +1167,7 @@ def stdapi_sys_process_execute(request, response):
             proc_h.stdin = os.fdopen(master, 'wb')
             proc_h.stdout = os.fdopen(master, 'rb')
             proc_h.stderr = open(os.devnull, 'rb')
+            proc_h.ptyfd = slave
         else:
             proc_h = STDProcess(args, stdin=subprocess.PIPE, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
             proc_h.echo_protection = True
diff --git a/python/meterpreter/meterpreter.py b/python/meterpreter/meterpreter.py
index 44c73508..82dca6fb 100644
--- a/python/meterpreter/meterpreter.py
+++ b/python/meterpreter/meterpreter.py
@@ -639,13 +639,17 @@ class MeterpreterProcess(MeterpreterChannel):
         super(MeterpreterProcess, self).__init__()
 
     def close(self):
-        self.proc_h.kill()
-        if hasattr(self.proc_h.stdin, 'close'):
-            self.proc_h.stdin.close()
-        if hasattr(self.proc_h.stdout, 'close'):
-            self.proc_h.stdout.close()
-        if hasattr(self.proc_h.stderr, 'close'):
-            self.proc_h.stderr.close()
+        if self.proc_h.poll() is None:
+            self.proc_h.kill()
+        if self.proc_h.ptyfd is not None:
+            os.close(self.proc_h.ptyfd)
+        for stream in (self.proc_h.stdin, self.proc_h.stdout, self.proc_h.stderr):
+            if not hasattr(stream, 'close'):
+                continue
+            try:
+                stream.close()
+            except (IOError, OSError):
+                pass
 
     def is_alive(self):
         return self.proc_h.poll() is None
@@ -739,18 +743,26 @@ class MeterpreterSocketUDPClient(MeterpreterSocket):
 export(MeterpreterSocketUDPClient)
 
 class STDProcessBuffer(threading.Thread):
-    def __init__(self, std, is_alive):
-        threading.Thread.__init__(self)
+    def __init__(self, std, is_alive, name=None):
+        threading.Thread.__init__(self, name=name or self.__class__.__name__)
         self.std = std
         self.is_alive = is_alive
         self.data = bytes()
         self.data_lock = threading.RLock()
 
+    def _read1(self):
+        try:
+            return self.std.read(1)
+        except (IOError, OSError):
+            return bytes()
+
     def run(self):
-        for byte in iter(lambda: self.std.read(1), bytes()):
+        byte = self._read1()
+        while len(byte):
             self.data_lock.acquire()
             self.data += byte
             self.data_lock.release()
+            byte = self._read1()
 
     def is_read_ready(self):
         return len(self.data) != 0
@@ -778,14 +790,15 @@ class STDProcess(subprocess.Popen):
         debug_print('[*] starting process: ' + repr(args[0]))
         subprocess.Popen.__init__(self, *args, **kwargs)
         self.echo_protection = False
+        self.ptyfd = None
 
     def is_alive(self):
         return self.poll() is None
 
     def start(self):
-        self.stdout_reader = STDProcessBuffer(self.stdout, self.is_alive)
+        self.stdout_reader = STDProcessBuffer(self.stdout, self.is_alive, name='STDProcessBuffer.stdout')
         self.stdout_reader.start()
-        self.stderr_reader = STDProcessBuffer(self.stderr, self.is_alive)
+        self.stderr_reader = STDProcessBuffer(self.stderr, self.is_alive, name='STDProcessBuffer.stderr')
         self.stderr_reader.start()
 
     def write(self, channel_data):
@@ -1273,15 +1286,15 @@ class PythonMeterpreter(object):
                 data = bytes()
                 write_request_parts = []
                 if isinstance(channel, MeterpreterProcess):
-                    if not channel_id in self.interact_channels:
-                        continue
-                    proc_h = channel.proc_h
-                    if proc_h.stderr_reader.is_read_ready():
-                        data = proc_h.stderr_reader.read()
-                    elif proc_h.stdout_reader.is_read_ready():
-                        data = proc_h.stdout_reader.read()
-                    elif not channel.is_alive():
+                    if channel_id in self.interact_channels:
+                        proc_h = channel.proc_h
+                        if proc_h.stderr_reader.is_read_ready():
+                            data = proc_h.stderr_reader.read()
+                        elif proc_h.stdout_reader.is_read_ready():
+                            data = proc_h.stdout_reader.read()
+                    if not channel.is_alive():
                         self.handle_dead_resource_channel(channel_id)
+                        channel.close()
                 elif isinstance(channel, MeterpreterSocketTCPClient):
                     while select.select([channel.fileno()], [], [], 0)[0]:
                         try:
@@ -1552,7 +1565,6 @@ class PythonMeterpreter(object):
         status, response = channel.core_eof(request, response)
         return ERROR_SUCCESS, response
 
-
     def _core_channel_interact(self, request, response):
         channel_id = packet_get_tlv(request, TLV_TYPE_CHANNEL_ID)['value']
         if channel_id not in self.channels: