[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

miss-islington  added the comment:


New changeset ad83fde75463dad2df878ff264f52436eb48bc6b by Miss Islington (bot) 
in branch '3.9':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
https://github.com/python/cpython/commit/ad83fde75463dad2df878ff264f52436eb48bc6b


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

miss-islington  added the comment:


New changeset 1a5001c606b55226c03fa1046aa8f5e1db2fa67d by Miss Islington (bot) 
in branch '3.8':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
https://github.com/python/cpython/commit/1a5001c606b55226c03fa1046aa8f5e1db2fa67d


--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

Change by miss-islington :


--
pull_requests: +23596
pull_request: https://github.com/python/cpython/pull/24831

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

Change by miss-islington :


--
pull_requests: +23595
pull_request: https://github.com/python/cpython/pull/24830

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

Change by miss-islington :


--
pull_requests: +23594
pull_request: https://github.com/python/cpython/pull/24823

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[Gregory P. Smith]

Gregory P. Smith  added the comment:

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.

--
resolution:  -> fixed
stage: patch review -> commit review
status: open -> closed

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[miss-islington]

Change by miss-islington :


--
nosy: +miss-islington
nosy_count: 7.0 -> 8.0
pull_requests: +23589
pull_request: https://github.com/python/cpython/pull/24824

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[Gregory P. Smith]

Gregory P. Smith  added the comment:


New changeset b4fc44bb2d209182390b4f9fdf074a46b0165a2f by Chris Griffith in 
branch 'master':
bpo-43423 Fix IndexError in subprocess _communicate function (GH-24777)
https://github.com/python/cpython/commit/b4fc44bb2d209182390b4f9fdf074a46b0165a2f


--
nosy: +gregory.p.smith

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[Eryk Sun]

Eryk Sun  added the comment:

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.

--
components: +Windows
nosy: +eryksun, paul.moore, steve.dower, tim.golden, zach.ware

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[Chris Griffith]

Change by Chris Griffith :


--
keywords: +patch
pull_requests: +23542
stage:  -> patch review
pull_request: https://github.com/python/cpython/pull/24777

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue43423] Subprocess IndexError possible in _communicate

[Chris Griffith]

New submission from Chris Griffith :

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.

--
components: Library (Lib)
messages: 388211
nosy: cdgriffith
priority: normal
severity: normal
status: open
title: Subprocess IndexError possible in _communicate
type: crash
versions: Python 3.10, Python 3.8, Python 3.9

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com