homepage

This issue tracker has been migrated to GitHub , and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: [Security] support.args_from_interpreter_flags() doesn't inherit -I (isolated) flag
Type: security Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: danishprakash, miss-islington, vstinner, xtreak
Priority: normal Keywords: patch

Created on 2018年09月26日 16:07 by vstinner, last changed 2022年04月11日 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 10675 merged vstinner, 2018年11月23日 14:01
PR 10684 merged miss-islington, 2018年11月23日 16:54
PR 10688 merged vstinner, 2018年11月23日 17:10
Messages (19)
msg326478 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年09月26日 16:07
The support.args_from_interpreter_flags() function recreates Python command line arguments from sys.flags, but it omits -I (sys.flags.isolated).
Because of that, "./python -I -m test ..." behaves differently than "./python -I -m test -j0 ...":
https://bugs.python.org/issue28655#msg326477 
msg326669 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018年09月29日 07:09
Thanks Victor for the details. Can this be classified as an easy issue? I guess the fix will be as below : 
1. Add an entry for '-I' at https://github.com/python/cpython/blob/4b430e5f6954ef4b248e95bfb4087635dcdefc6d/Lib/subprocess.py#L260
2. Add a test for '-I' at https://github.com/python/cpython/blob/4b430e5f6954ef4b248e95bfb4087635dcdefc6d/Lib/test/test_support.py#L472. 
The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options. check_options should be changed so that this can take given args and the expected args for comparison to accommodate -I. Maybe there is a better way?
Off topic : I don't know why '-I' is not documented as sys.flags.isolated at https://docs.python.org/3.7/library/sys.html#sys.flags . Maybe I will open up a separate issue for this?
msg326787 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年10月01日 10:21
In the C code, sys.flags.isolated clearly documented as linked to the -I option:
static PyStructSequence_Field flags_fields[] = {
 {"debug", "-d"},
 {"inspect", "-i"},
 {"interactive", "-i"},
 {"optimize", "-O or -OO"},
 {"dont_write_bytecode", "-B"},
 {"no_user_site", "-s"},
 {"no_site", "-S"},
 {"ignore_environment", "-E"},
 {"verbose", "-v"},
 /* {"unbuffered", "-u"}, */
 /* {"skip_first", "-x"}, */
 {"bytes_warning", "-b"},
 {"quiet", "-q"},
 {"hash_randomization", "-R"},
 {"isolated", "-I"},
 {"dev_mode", "-X dev"},
 {"utf8_mode", "-X utf8"},
 {0}
};
> The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options.
I expect to get:
$ python3 -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())'
['-I']
instead of:
['-s', '-E']
-I is different from -s -E: it also avoids to add the script directory or an empty string to sys.path.
msg326790 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018年10月01日 10:36
Thanks Victor for the details. 
> In the C code, sys.flags.isolated clearly documented as linked to the -I option:
With respect to documentation I was talking about '-I' not being documented in the table at https://docs.python.org/3.7/library/sys.html#sys.flags though it's present in the C code and in sys.flags.isolated.
> -I is different from -s -E: it also avoids to add the script directory or an empty string to sys.path.
'-I' also implies '-s -E' and hence adding isolated to args_from_interpreter_flags will also return ['-s', '-E', '-I'] as output and hence I suggested modifying the comparison logic.
# Since '-I' implies '-s' and '-E' those flags are also set returning '-s -E -I'
./python.exe --help | rg '\-I'
-I : isolate Python from the user's environment (implies -E and -s)
./python.exe -I -c 'import sys; print(sys.flags)'
sys.flags(debug=0, inspect=0, interactive=0, optimize=0, dont_write_bytecode=0, no_user_site=1, no_site=0, ignore_environment=1, verbose=0, bytes_warning=0, quiet=0, hash_randomization=1, isolated=1, dev_mode=False, utf8_mode=0)
# patching args_from_interpreter_flags to support '-I' would return below
./python.exe -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())'
['-s', '-E', '-I']
Thanks
msg326792 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年10月01日 11:10
> ./python.exe -I -c 'import subprocess; print(subprocess._args_from_interpreter_flags())'
> ['-s', '-E', '-I']
This looks wrong, I would prefer to only get ['-I'].
msg326875 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年10月02日 12:16
> With respect to documentation I was talking about '-I' not being documented in the table at https://docs.python.org/3.7/library/sys.html#sys.flags though it's present in the C code and in sys.flags.isolated.
Thanks for bringing this up Karthikeyan, however, could there be another reason why -I would be left out. Also, have you filed an issue for this?
Also, Victor and Karthikeyan, since this issue has been categorized as an easy issue, I would like to fix this if none of you have started working on this.
msg326876 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018年10月02日 12:25
> Thanks for bringing this up Karthikeyan, however, could there be another reason why -I would be left out. Also, have you filed an issue for this?
I couldn't see any related issue for this though the table was changed in 3.7.0
> Also, Victor and Karthikeyan, since this issue has been categorized as an easy issue, I would like to fix this if none of you have started working on this.
I am not working on this. Feel free to pick it up.
msg326879 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年10月02日 12:46
Thank you Karthikeyan, I'm going to take care of both of these issues.
msg327099 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年10月05日 04:28
Linking this[1] here in case someone else stumbles upon this thread. I've created an issue and a PR for the documentation issue regarding the absence of -I flag from the sys.flags table which came into picture from the discussions in this thread.
[1]: https://bugs.python.org/issue34901 
msg327280 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年10月07日 13:40
> I expect to get: ['-I'] instead of ['-s', '-E', '-I']
From what I understand, this can be done in one of two ways. First, we could edit https://github.com/python/cpython/blob/ad73a9cf97770023665a1bb1c6390a3c99478139/Modules/main.c#L430 and not incrementing -s and -E. But I believe this would have consequences that I'm unable to think of right now, so I'd like some inputs on this. 
Secondly, we could handle this condition in `_args_from_interpreted_flags()` itself but that might be looked upon as bad practice.
msg327289 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018年10月07日 16:47
> From what I understand, this can be done in one of two ways. First, we could edit https://github.com/python/cpython/blob/ad73a9cf97770023665a1bb1c6390a3c99478139/Modules/main.c#L430 and not incrementing -s and -E. But I believe this would have consequences that I'm unable to think of right now, so I'd like some inputs on this. 
As in the docs for -I it implies -s and -E so removing the increment is not a good solution in my opinion and will break code. 
I don't know how this can be handled since -I sets -s and -E implicitly and _args_from_interpreted_flags just looks for the set flag. This could also get a little complex if we remove -s and -E based on -I since one might pass -I and -s. Maybe we can do an intersection of the command line arguments passes and the set bits in _args_from_interpreted_flags so that only -I remains? Victor prefers -I only and maybe has an approach to solve this?
msg327318 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年10月08日 03:37
You're right Karthikeyan, although I personally think that returning ['-s', '-E', '-I'] should be a plausible solution here since it has been stated explicitly that it implies '-s' and '-E' but I'm still waiting for what Victor has to say on this. 
> The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options.
Karthikeyan, do you happen to have a use case where this might come into action?
msg327321 - (view) Author: Karthikeyan Singaravelan (xtreak) * (Python committer) Date: 2018年10月08日 04:08
>> The only thing here is that '-I' returns '-s -E -I' unlike other options where args can be used for comparison logic in check_options.
> Karthikeyan, do you happen to have a use case where this might come into action?
I don't have a use case in mind. The comment was that returning '-s -E -I' would need the helper function used in the test to be changed.
Thanks
msg330295 - (view) Author: Danish Prakash (danishprakash) * Date: 2018年11月23日 03:02
Sorry for bumping this thread but Victor, could you please share your inputs on this if you have the time for it, thanks.
msg330325 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年11月23日 14:03
I tried to explain how to fix the bug, but nobody came up with a working change 2 months, so I wrote the PR myself. It's an important security issue, since the function is used by multiprocessing and distutils modules to spawn new child processes.
msg330340 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年11月23日 16:54
New changeset 9de363271519e0616f4a7b59427057c4810d3acc by Victor Stinner in branch 'master':
bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675)
https://github.com/python/cpython/commit/9de363271519e0616f4a7b59427057c4810d3acc
msg330344 - (view) Author: miss-islington (miss-islington) Date: 2018年11月23日 17:13
New changeset 01e579949ab546cd4cdd0d6d18e3ef41ce94f46e by Miss Islington (bot) in branch '3.7':
bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675)
https://github.com/python/cpython/commit/01e579949ab546cd4cdd0d6d18e3ef41ce94f46e
msg330351 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年11月23日 18:02
New changeset cc0e0a2214d6515cf6ba4c7b164902a87e321b45 by Victor Stinner in branch '3.6':
bpo-34812: subprocess._args_from_interpreter_flags(): add isolated (GH-10675) (GH-10688)
https://github.com/python/cpython/commit/cc0e0a2214d6515cf6ba4c7b164902a87e321b45
msg330352 - (view) Author: STINNER Victor (vstinner) * (Python committer) Date: 2018年11月23日 18:03
Ok, the bug is now fixed in Python 3.6, 3.7 and master branches ;-)
History
Date User Action Args
2022年04月11日 14:59:06adminsetgithub: 78993
2018年11月23日 18:03:21vstinnersetstatus: open -> closed
resolution: fixed
messages: + msg330352

stage: patch review -> resolved
2018年11月23日 18:02:28vstinnersetmessages: + msg330351
2018年11月23日 17:13:35miss-islingtonsetnosy: + miss-islington
messages: + msg330344
2018年11月23日 17:10:22vstinnersetpull_requests: + pull_request9940
2018年11月23日 16:54:37miss-islingtonsetpull_requests: + pull_request9936
2018年11月23日 16:54:23vstinnersetmessages: + msg330340
2018年11月23日 14:03:59vstinnersettitle: [EASY] support.args_from_interpreter_flags() doesn't inherit -I (isolated) flag -> [Security] support.args_from_interpreter_flags() doesn't inherit -I (isolated) flag
messages: + msg330325

components: - Tests
keywords: - easy
type: security
2018年11月23日 14:01:48vstinnersetkeywords: + patch
stage: patch review
pull_requests: + pull_request9931
2018年11月23日 03:02:43danishprakashsetmessages: + msg330295
2018年10月08日 04:08:45xtreaksetmessages: + msg327321
2018年10月08日 03:37:37danishprakashsetmessages: + msg327318
2018年10月07日 16:47:59xtreaksetmessages: + msg327289
2018年10月07日 13:40:54danishprakashsetmessages: + msg327280
2018年10月05日 04:28:19danishprakashsetmessages: + msg327099
2018年10月02日 12:46:14danishprakashsetmessages: + msg326879
2018年10月02日 12:25:36xtreaksetmessages: + msg326876
2018年10月02日 12:16:22danishprakashsetnosy: + danishprakash
messages: + msg326875
2018年10月01日 11:10:17vstinnersetmessages: + msg326792
2018年10月01日 10:36:37xtreaksetmessages: + msg326790
2018年10月01日 10:25:43vstinnersetkeywords: + easy
title: support.args_from_interpreter_flags() doesn't inherit -I (isolated) flag -> [EASY] support.args_from_interpreter_flags() doesn't inherit -I (isolated) flag
2018年10月01日 10:21:45vstinnersetmessages: + msg326787
2018年09月29日 07:09:30xtreaksetmessages: + msg326669
2018年09月26日 16:10:02xtreaksetnosy: + xtreak
2018年09月26日 16:07:03vstinnercreate

AltStyle によって変換されたページ (->オリジナル) /