-
-
Notifications
You must be signed in to change notification settings - Fork 32k
bpo-40932: Note security caveat of shlex.quote on Windows #21502
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
bpo-40932: Note security caveat of shlex.quote on Windows #21502
Conversation
619975f
to
20cea90
Compare
See my comment on the issue, but if we decide that dividing by OS is clear enough (even though there are non-conforming shells on both OSs) then this is fine. |
It's a worthy note indeed. It's not only security: it'll likely fail or work incorrectly with a single quote A similar function for Windows is |
Adjusted wording for the shlex warning based on feedback on the bug. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other than a spelling typo, I think this looks good now.
Co-authored-by: Zachary Ware <[email protected]>
Doc/library/shlex.rst
Outdated
|
||
The ``shlex`` module is **only designed for Unix shells**. | ||
|
||
The :func:`quote` function is not guaranteed to be safe on non-POSIX |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is there such a thing as a POSIX-compliant shell? Also I think "guaranteed to be correct" is better than "safe", here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a POSIX-compliant shell
I think it's a pretty common term I've seen, loosely meaning shells that follow the posix specs here https://pubs.opengroup.org/onlinepubs/009695399/utilities/contents.html
Your wording definitely seems a bit better, changed.
…1502) Added a note in the `subprocess` docs that recommend using `shlex.quote` without mentioning that this is only applicable to Unix. Also added a warning straight into the `shlex` docs since it only says "for simple syntaxes resembling that of the Unix shell" and says using `quote` plugs the security hole without mentioning this important caveat.
Added a note in the
subprocess
docs that recommend usingshlex.quote
without mentioning that this is only applicable to Unix.Also added a warning straight into the
shlex
docs since it only says "for simple syntaxes resembling that of the Unix shell" and says usingquote
plugs the security hole without mentioning this important caveat.https://bugs.python.org/issue40932
Automerge-Triggered-By: GH:zware