msg215020 - (view) |
Author: Ryan Lortie (desrt) |
Date: 2014-03-28 07:04 |
http://bugs.python.org/file19849/mkdirs.tr.diff introduced a patch with this code in it:
+def _get_masked_mode(mode):
+ mask = umask(0)
+ umask(mask)
+ return mode & ~mask
This changes the umask of the entire process. If another thread manages to create a file between those two calls then it will be world read/writable, regardless of the original umask of the process.
This is not theoretical: I discovered this bug by observing it happen.
|
msg215022 - (view) |
Author: Ned Deily (ned.deily) * |
Date: 2014-03-28 08:19 |
The issue associated with the patch in question is Issue9299. Adding possibly affected releases and release managers for evaluation.
|
msg215024 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2014-03-28 08:54 |
yes, this seems bad enough for inclusion in security releases.
|
msg215025 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-03-28 09:02 |
> http://bugs.python.org/file19849/mkdirs.tr.diff
This patch comes from issue #9299: changeset 89cda0833ba6 made in Python 3.2 beta 1. The change was not backported to Python 2.7.
|
msg215026 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-03-28 09:10 |
The shell command "umask" calls umask(022) to get the current umask, and then call umask() with result of the first call.
022 is the default umask, it's probably safer to call umask(0o22) in _get_masked_mode() instead of umask(0).
Attached patch makes this change.
If you change something, it should be backported to 3.2, 3.3 and 3.4, because I agree that it affects the security.
|
msg215027 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-03-28 09:47 |
I think the behaviour that an error is raised if the permissions are not the same is a nuisance that does not correspond to actual use cases (*). People who care about permissions so much that they expect an error can do the check themselves, or call chmod().
(*) and I got similar errors several times when running setup.py, only I didn't know it was because of that "feature"
|
msg215028 - (view) |
Author: Antoine Pitrou (pitrou) * |
Date: 2014-03-28 09:48 |
(note that Victor's patch is of course not an actual fix, only a mitigation; if someone is relying on a stricter umask they will still be vulnerable to this)
|
msg215034 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2014-03-28 10:26 |
> I think the behaviour that an error is raised if the permissions are not the same is a nuisance that does not correspond to actual use cases (*).
I was also surprised that makedirs() checks for the exact permission.
We can probably document that makedirs(exists_ok=True) leaves the
directory permission unchanged if the directory already exist, and
that an explicit chmod() may be needed to ensure that permissions are
the expected permissions.
If the check on permissions is removed, an enhancement would be to
return a flag to indicate if at least one directory of the path
already existed. So the caller can avoid calling chmod() if all
directories of the path had to be created.
Something like:
if makedirs("a/b", mod=0o755, exists_ok=True):
os.chmod("a", 0o755)
os.chmod("a/b", 0o755)
# else a and b were created with the permission 0o755
|
msg215229 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2014-03-31 11:38 |
See also issue19930. Behaviors of os.makedirs() and pathlib.Path.mkdir() are different. pathlib.Path.mkdir() (as the mkdir command) creates parent directories with default mode, and os.makedirs() - with specified mode.
|
msg215345 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2014-04-01 23:22 |
New changeset 9186f4a18584 by Benjamin Peterson in branch '3.2':
remove directory mode check from makedirs (closes #21082)
http://hg.python.org/cpython/rev/9186f4a18584
New changeset 6370d44013f7 by Benjamin Peterson in branch '3.3':
merge 3.2 (#21082)
http://hg.python.org/cpython/rev/6370d44013f7
New changeset c24dd53ab4b9 by Benjamin Peterson in branch '3.4':
merge 3.3 (#21082)
http://hg.python.org/cpython/rev/c24dd53ab4b9
New changeset adfcdc831e98 by Benjamin Peterson in branch 'default':
merge 3.4 (#21082)
http://hg.python.org/cpython/rev/adfcdc831e98
|
msg215346 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2014-04-01 23:23 |
I've now removed the offending behavior. exist_ok is still racy because it uses path.isdir() in the exceptional case, but fixing that can be an enhancement elsewhere.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:58:00 | admin | set | github: 65281 |
2021-11-04 14:29:38 | eryksun | set | messages:
- msg405701 |
2021-11-04 14:29:24 | eryksun | set | nosy:
- ahmedsayeed1982 components:
+ Library (Lib), - Subinterpreters
|
2021-11-04 12:11:02 | ahmedsayeed1982 | set | versions:
- Python 3.3, Python 3.4, Python 3.5 nosy:
+ ahmedsayeed1982, - terry.reedy, pitrou, vstinner, larry, christian.heimes, matejcik, benjamin.peterson, ned.deily, Arfrever, flox, python-dev, rpointel, serhiy.storchaka, koobs, desrt
messages:
+ msg405701
components:
+ Subinterpreters, - Library (Lib) |
2014-04-01 23:23:44 | benjamin.peterson | set | nosy:
+ benjamin.peterson messages:
+ msg215346
|
2014-04-01 23:22:27 | python-dev | set | status: open -> closed
nosy:
+ python-dev messages:
+ msg215345
resolution: fixed stage: resolved |
2014-03-31 15:06:01 | matejcik | set | nosy:
+ matejcik
|
2014-03-31 11:38:06 | serhiy.storchaka | set | nosy:
+ serhiy.storchaka messages:
+ msg215229
|
2014-03-31 02:11:32 | rhettinger | set | nosy:
+ christian.heimes
|
2014-03-28 22:39:19 | flox | set | nosy:
+ flox
|
2014-03-28 21:52:25 | koobs | set | nosy:
+ koobs
|
2014-03-28 18:02:17 | terry.reedy | set | nosy:
+ Arfrever
|
2014-03-28 16:58:45 | benjamin.peterson | set | priority: high -> release blocker |
2014-03-28 14:14:08 | rpointel | set | nosy:
+ rpointel
|
2014-03-28 10:26:39 | vstinner | set | messages:
+ msg215034 |
2014-03-28 09:48:45 | pitrou | set | messages:
+ msg215028 |
2014-03-28 09:47:27 | pitrou | set | nosy:
+ terry.reedy, pitrou messages:
+ msg215027
|
2014-03-28 09:10:53 | vstinner | set | files:
+ get_masked_mode.patch keywords:
+ patch messages:
+ msg215026
|
2014-03-28 09:04:04 | vstinner | set | title: os.makedirs() is not thread-safe: umask is set temporary to 0, serious security problem -> os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem |
2014-03-28 09:03:03 | vstinner | set | title: _get_masked_mode in os.makedirs() is a serious security problem -> os.makedirs() is not thread-safe: umask is set temporary to 0, serious security problem |
2014-03-28 09:02:06 | vstinner | set | messages:
+ msg215025 |
2014-03-28 08:59:13 | vstinner | set | nosy:
+ vstinner
|
2014-03-28 08:54:24 | georg.brandl | set | messages:
+ msg215024 |
2014-03-28 08:19:54 | ned.deily | set | priority: normal -> high versions:
+ Python 3.2, Python 3.4, Python 3.5 nosy:
+ georg.brandl, larry, ned.deily
messages:
+ msg215022
keywords:
+ security_issue |
2014-03-28 07:04:06 | desrt | create | |