[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Eryk Sun]

Change by Eryk Sun :


--
Removed message: https://bugs.python.org/msg405701

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Eryk Sun]

Change by Eryk Sun :


--
components: +Library (Lib) -Subinterpreters
nosy:  -ahmedsayeed1982

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Ahmed Sayeed]

Ahmed Sayeed  added the comment:

[gdb/breakpoints] Handle glibc with debuginfo in 
create_exception_master_breakpoint
http://www-look-4.com/computers/huawei-computers/
The test-case nextoverthrow.exp is failing on targets with unstripped libc.
https://komiya-dental.com/category/services/
This is a regression since commit 1940319c0ef "[gdb] Fix internal-error in
process_event_stop_test".
http://www.iu-bloomington.com/category/services/
The problem is that this code in create_exception_master_breakpoint:
...
  for (objfile *sepdebug = obj->separate_debug_objfile; 
https://waytowhatsnext.com/category/services/
   sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile)
if (create_exception_master_breakpoint_hook (sepdebug))
... http://www.wearelondonmade.com/computers/car-computers/
iterates over all the separate debug object files, but fails to handle the
case that obj itself has the debug info we're looking for.
http://www.jopspeech.com/health/nvidia-rtx-2060/
Fix this by using the separate_debug_objfiles () range instead, which does
iterate both over obj and the obj->separate_debug_objfile chain. 
http://joerg.li/health/xiaomi/

Tested on x86_64-linux.
http://connstr.net/computers/chargers-tech/
gdb/ChangeLog:
[gdb/breakpoints] Handle glibc with debuginfo in 
create_exception_master_breakpoint
http://embermanchester.uk/computers/video-conversation/
The test-case nextoverthrow.exp is failing on targets with unstripped libc.
http://www.slipstone.co.uk/computers/isofix/
This is a regression since commit 1940319c0ef "[gdb] Fix internal-error in
process_event_stop_test". http://www.logoarts.co.uk/computers/printer-types/

The problem is that this code in create_exception_master_breakpoint:
... http://www.acpirateradio.co.uk/technology/facetime/
  for (objfile *sepdebug = obj->separate_debug_objfile;
   sepdebug != nullptr; sepdebug = sepdebug->separate_debug_objfile)
if (create_exception_master_breakpoint_hook (sepdebug)) 
http://www.compilatori.com/computers/smartphones/
...
iterates over all the separate debug object files, but fails to handle the
case that obj itself has the debug info we're looking for.
 https://www.webb-dev.co.uk/category/services/
Fix this by using the separate_debug_objfiles () range instead, which does
iterate both over obj and the obj->separate_debug_objfile chain.

Tested on x86_64-linux.

gdb/ChangeLog:

--
components: +Subinterpreters -Library (Lib)
nosy: +ahmedsayeed1982 -Arfrever, benjamin.peterson, christian.heimes, desrt, 
flox, koobs, larry, matejcik, ned.deily, pitrou, python-dev, rpointel, 
serhiy.storchaka, terry.reedy, vstinner
versions:  -Python 3.3, Python 3.4, Python 3.5

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Roundup Robot]

Roundup Robot added the comment:

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

--
nosy: +python-dev
resolution:  - fixed
stage:  - committed/rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Benjamin Peterson]

Benjamin Peterson added the comment:

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.

--
nosy: +benjamin.peterson

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

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.

--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[jan matejek]

Changes by jan matejek jmate...@suse.cz:


--
nosy: +matejcik

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Raymond Hettinger]

Changes by Raymond Hettinger raymond.hettin...@gmail.com:


--
nosy: +christian.heimes

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[STINNER Victor]

Changes by STINNER Victor victor.stin...@gmail.com:


--
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

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[STINNER Victor]

STINNER Victor added the comment:

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.

--
keywords: +patch
Added file: http://bugs.python.org/file34649/get_masked_mode.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Antoine Pitrou]

Antoine Pitrou added the comment:

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

--
nosy: +pitrou, terry.reedy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Antoine Pitrou]

Antoine Pitrou added the comment:

(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)

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[STINNER Victor]

STINNER Victor added the comment:

 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

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Remi Pointel]

Changes by Remi Pointel pyt...@xiri.fr:


--
nosy: +rpointel

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Benjamin Peterson]

Changes by Benjamin Peterson benja...@python.org:


--
priority: high - release blocker

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Terry J. Reedy]

Changes by Terry J. Reedy tjre...@udel.edu:


--
nosy: +Arfrever

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[koobs]

Changes by koobs koobs.free...@gmail.com:


--
nosy: +koobs

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21082] os.makedirs(exist_ok=True) is not thread-safe: umask is set temporary to 0, serious security problem

[Florent Xicluna]

Changes by Florent Xicluna florent.xicl...@gmail.com:


--
nosy: +flox

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21082
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com