Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix python meterpreter subprocess deadlock and file descriptor leak #653

Conversation

adfoster-r7
Copy link
Contributor

Depends on #651 being merged first

Fixes the following resource leak with subprocess.popen - caused by the stdout/stderr file descriptors not being closed:

$ python3 -m unittest discover -v ./tests
test_stdapi_net_config_get_interfaces (test_ext_server_stdapi.TestExtServerStdApi.test_stdapi_net_config_get_interfaces) ... <string>:2183: ResourceWarning: unclosed file <_io.BufferedReader name=3>
Object allocated at (most recent call last):
  File "/usr/local/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/subprocess.py", lineno 1014
    self.stdout = io.open(c2pread, 'rb', bufsize)
<string>:2183: ResourceWarning: unclosed file <_io.BufferedReader name=5>
Object allocated at (most recent call last):
  File "/usr/local/Cellar/python@3.11/3.11.2_1/Frameworks/Python.framework/Versions/3.11/lib/python3.11/subprocess.py", lineno 1019
    self.stderr = io.open(errread, 'rb', bufsize)

Also fixes an edgecase with deadlocking when processes return large output:

Popen.wait(timeout=None)
Wait for child process to terminate. Set and return returncode attribute.

Note This will deadlock when using stdout=PIPE or stderr=PIPE and the child process generates enough output to a pipe such that it blocks waiting for the OS pipe buffer to accept more data. Use Popen.communicate() when using pipes to avoid that.

Relates to #569

@adfoster-r7 adfoster-r7 force-pushed the fix-python-meterpreter-subprocess-deadlock-and-file-descriptor-leak branch from 24ddd0a to 5177490 Compare June 14, 2023 09:53
@@ -879,6 +879,14 @@ def ctstruct_unpack(structure, raw_data):
ctypes.memmove(ctypes.byref(structure), raw_data, ctypes.sizeof(structure))
return structure

def get_process_output(args):
proc_h = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
stdout, stderr = proc_h.communicate()
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Method manually tested with 2.4, as I couldn't get pip/mock installed with an older version of python

# cat test.py 
import subprocess

def get_process_output(args):
    proc_h = subprocess.Popen(args, stdout=subprocess.PIPE, stderr=subprocess.PIPE)
    stdout, stderr = proc_h.communicate()

    if proc_h.wait():
        raise Exception(args[0] + ' exited with non-zero status')
    return str(stdout)


print(get_process_output(['ps', 'ax', '-w', '-o', 'pid,ppid,user,command']))
# python2.4 test.py 
  PID  PPID USER     COMMAND
    1     0 root     /bin/bash
 2157     1 root     python2.4 test.py
 2158  2157 root     ps ax -w -o pid,ppid,user,command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants