classification
Title: os.popen & os.system lack shell-related security warnings
Type: Stage:
Components: Documentation Versions: Python 3.5, Python 2.7
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: docs@python Nosy List: cvrebert, docs@python, r.david.murray, rhettinger, vstinner
Priority: normal Keywords: patch

Created on 2014-05-22 21:33 by cvrebert, last changed 2015-02-13 01:25 by demian.brecht.

Files
File name Uploaded Description Edit
fix-21557.patch cvrebert, 2014-12-01 20:22 Adds shell insecurity warnings to the os module docs review
Messages (7)
msg218921 - (view) Author: Chris Rebert (cvrebert) * Date: 2014-05-22 21:33
Since these functions run shell commands, which is a common vector for security-related bugs (see
* http://cwe.mitre.org/data/definitions/78.html
* http://cwe.mitre.org/data/definitions/88.html
), I suggest that they should have security warning boxes analogous to the one for the `subprocess` module:
https://docs.python.org/2/library/subprocess.html#frequently-used-arguments
msg231953 - (view) Author: Chris Rebert (cvrebert) * Date: 2014-12-01 20:22
Here is a patch that adds the necessary warnings from issue 7950.
Please review it when you get a chance.
msg231958 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2014-12-01 22:46
Left a comment in Rietveld.
msg232089 - (view) Author: Demian Brecht (demian.brecht) * (Python triager) Date: 2014-12-03 19:48
After discussion in Rietveld, the patch looks good to me.
msg232093 - (view) Author: R. David Murray (r.david.murray) * (Python committer) Date: 2014-12-03 21:32
Since Raymond is the person who tends to object most strongly to warning boxes in the docs, let's get his opinion on this.  I'm not sure that the warning box is necessary, the text may be sufficient.  On the other hand, this *is* a significant insecurity vector.

As far as the text goes, I'd combine the two paragraphs and introduce the text from the second one with "Alternatively, ...".  And if it isn't a warning box, the the language should be refocused to be positive: "Use the Popen module with shell=False to avoid the common security issues involved in using unsanitized input from untrusted sources..."
msg232211 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-12-05 21:37
Python 3.5 doc has no red warning in the subprocess module, but a nice section:
https://docs.python.org/dev/library/subprocess.html#security-considerations

Why not simply copying the note of the subprocess doc?
"Note: Read the Security Considerations section before using shell=True."
msg232213 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2014-12-05 21:38
subprocess.getstatusoutput(cmd) needs also this note (or warning).
History
Date User Action Args
2015-02-13 01:25:53demian.brechtsetnosy: - demian.brecht
2014-12-05 21:38:35vstinnersetmessages: + msg232213
2014-12-05 21:37:40vstinnersetmessages: + msg232211
2014-12-05 21:33:59vstinnersetnosy: + vstinner
2014-12-03 21:32:25r.david.murraysetnosy: + rhettinger, r.david.murray
messages: + msg232093
2014-12-03 19:48:41demian.brechtsetmessages: + msg232089
2014-12-01 22:46:42demian.brechtsetnosy: + demian.brecht
messages: + msg231958
2014-12-01 20:22:12cvrebertsetfiles: + fix-21557.patch
keywords: + patch
messages: + msg231953
2014-05-22 21:33:48cvrebertcreate