Subprocess IndexError possible in _communicate

[cdgriffith]

BPO	43423
Nosy	@gpshead, @pfmoore, @tjguk, @zware, @eryksun, @zooba, @cdgriffith, @miss-islington
PRs	bpo-43423 Fix IndexError in subprocess _communicate function #24777 [3.8] bpo-43423: Fix IndexError in subprocess _communicate function (GH-24777) #24824 [3.9] bpo-43423: Fix IndexError in subprocess _communicate function (GH-24777) #24823 [3.8] bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777) #24830 [3.9] bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777) #24831

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2021-03-11.19:51:50.937>
created_at = <Date 2021-03-06.18:52:42.452>
labels = ['3.8', 'type-crash', '3.10', 'library', 'OS-windows', '3.9']
title = 'Subprocess IndexError possible in _communicate'
updated_at = <Date 2021-03-12.01:56:49.605>
user = 'https://github.com/cdgriffith'

bugs.python.org fields:

activity = <Date 2021-03-12.01:56:49.605>
actor = 'miss-islington'
assignee = 'none'
closed = True
closed_date = <Date 2021-03-11.19:51:50.937>
closer = 'gregory.p.smith'
components = ['Library (Lib)', 'Windows']
creation = <Date 2021-03-06.18:52:42.452>
creator = 'cdgriffith'
dependencies = []
files = []
hgrepos = []
issue_num = 43423
keywords = ['patch']
message_count = 6.0
messages = ['388211', '388213', '388520', '388521', '388531', '388533']
nosy_count = 8.0
nosy_names = ['gregory.p.smith', 'paul.moore', 'tim.golden', 'zach.ware', 'eryksun', 'steve.dower', 'cdgriffith', 'miss-islington']
pr_nums = ['24777', '24824', '24823', '24830', '24831']
priority = 'normal'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = 'crash'
url = 'https://bugs.python.org/issue43423'
versions = ['Python 3.8', 'Python 3.9', 'Python 3.10']

[cdgriffith]

It is possible to run into an IndexError in the subprocess module's _communicate function.

    return run(
  File "subprocess.py", line 491, in run
  File "subprocess.py", line 1024, in communicate
  File "subprocess.py", line 1418, in _communicate
IndexError: list index out of range

The lines in question are:

            if stdout is not None:
                stdout = stdout[0]
            if stderr is not None:
                stderr = stderr[0]

I believe this is due to the fact there is no safety checking to make sure that self._stdout_buff and self._stderr_buff have any content in them after being set to empty lists.

The fix I suggest is to change the checks from if stdout is not None to simply if stdout to make sure it is a populated list.

Example fixed code:

            if stdout:
                stdout = stdout[0]
            if stderr:
                stderr = stderr[0]

If a more stringent check is required, we could expand that out to check type and length, such as `isinstance(stdout, list) and len(stdout) > 0:` but that is more then necessary currently.

[eryksun]

The presumption I suppose is that these statements only execute if self.stdout_thread and/or self.stderr_thread completes successfully. I suppose that the read could fail or get canceled via CancelSynchronousIo(). Of course in that case you have a bigger problem than an IndexError.

On a related note, _communicate() needs significant changes in Windows. See bpo-43346 if you're interested.

[gpshead]

New changeset b4fc44b by Chris Griffith in branch 'master':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
b4fc44b

[gpshead]

The bug is fixed, Thanks Chris! There was a refactoring noted as being nice in my comments on the primary main branch PR that would be nice to have. But isn't critical. If you want to make a PR for that, just reuse this bpo-43423 issue number on the PR and assign it to me.

The backports to 3.9 an 3.8 are approved and should automerge after CI finishes.

[miss-islington]

New changeset 1a5001c by Miss Islington (bot) in branch '3.8':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
1a5001c

[miss-islington]

New changeset ad83fde by Miss Islington (bot) in branch '3.9':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
ad83fde