Created on 2020-06-09 20:49 by Stephen Farris, last changed 2021-04-02 15:56 by ammar2. This issue is now closed.
| Pull Requests | |||
|---|---|---|---|
| URL | Status | Linked | Edit |
| PR 21502 | merged | ammar2, 2020-07-16 03:09 | |
| Messages (5) | |||
|---|---|---|---|
| msg371140 - (view) | Author: Stephen Farris (Stephen Farris) | Date: 2020-06-09 20:49 | |
The subprocess docs state: "When using shell=True, the shlex.quote() function can be used to properly escape whitespace and shell metacharacters in strings that are going to be used to construct shell commands." While this is true on Unix, it is not true on Windows. On Windows it is easy to create scenarios where shell injection still exists despite using shlex.quote properly (e.g. subprocess.run(shlex.quote("'&calc '"), shell=True) launches the Windows calculator, which it wouldn't do if shlex.quote was able to prevent shell injection on Windows). While the shlex docs state that shlex is for Unix, the subprocess docs imply that shlex.quote will work on Windows too, possibly leading some developers to erroneously use shlex.quote on Windows to try to prevent shell injection. Recommend: 1) qualifying the above section in the subprocess docs to make it clear that this only works on Unix, and 2) updating the shlex docs with warnings that shlex.quote in particular is not for use on Windows.
|
|||
| msg374031 - (view) | Author: Steve Dower (steve.dower) *  |
Date: 2020-07-20 22:22 | |
I wonder whether we should be more specific about the shells that shlex works for? Since WSL makes *sh (Bash, Dash, Sh, etc.) easily available on Windows, and I believe PowerShell on Linux keeps its own quoting rules. |
|||
| msg374051 - (view) | Author: Ammar Askar (ammar2) *  |
Date: 2020-07-21 03:19 | |
Hmm, it'd be hard to enumerate them all. The module does say, "...simple syntaxes resembling that of the Unix shell" but that's it. Distinguishing at the OS level for shlex does seem a bit weird given the existence of WSL and non-compliant shells on Linux like xonsh. I think it'd be nice if we could be a bit more specific on whats supported, maybe it covers all POSIX compliant shells? For the subprocess warning I think it's fine to talk about the OS since it looks like the shell used are hard-coded in: * https://github.com/python/cpython/blob/5241e189e77972d3a07acbbb3f0c0cbc2aeeb681/Lib/subprocess.py#L1403-L1407 * https://github.com/python/cpython/blob/5241e189e77972d3a07acbbb3f0c0cbc2aeeb681/Lib/subprocess.py#L1680-L1686 |
|||
| msg380738 - (view) | Author: miss-islington (miss-islington) | Date: 2020-11-11 07:30 | |
New changeset f9a8386e44a695551a1e54e709969e90e9b96bc4 by Ammar Askar in branch 'master': bpo-40932: Note security caveat of shlex.quote on Windows (GH-21502) https://github.com/python/cpython/commit/f9a8386e44a695551a1e54e709969e90e9b96bc4 |
|||
| msg390075 - (view) | Author: Ammar Askar (ammar2) * |
Date: 2021-04-02 15:56 | |
Thank you Steve and Zachary for reviewing, this warning is in the docs now. |
|||
| History | |||
|---|---|---|---|
| Date | User | Action | Args |
| 2021-04-02 15:56:16 | ammar2 | set | status: open -> closed resolution: fixed messages: + msg390075 stage: patch review -> resolved |
| 2020-11-11 07:30:06 | miss-islington | set | nosy:
+ miss-islington messages: + msg380738 |
| 2020-07-21 03:19:45 | ammar2 | set | messages: + msg374051 |
| 2020-07-20 22:22:01 | steve.dower | set | messages: + msg374031 |
| 2020-07-16 03:09:33 | ammar2 | set | keywords:
+ patch nosy: + ammar2 pull_requests: + pull_request20643 stage: patch review |
| 2020-06-10 11:53:22 | chris.jerdonek | set | nosy:
+ chris.jerdonek |
| 2020-06-10 07:40:03 | ned.deily | set | nosy:
+ paul.moore, tim.golden, zach.ware, steve.dower title: subprocess docs don't qualify the instruction to use shlex.quote by OS -> subprocess docs should warn of shlex use on Windows components: + Windows versions: + Python 3.9, Python 3.10 |
| 2020-06-09 20:50:18 | Stephen Farris | set | type: security |
| 2020-06-09 20:49:51 | Stephen Farris | create | |