[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Tom Hale]

Change by Tom Hale :


--
type: security -> enhancement

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Tom Hale]

Tom Hale  added the comment:

Thanks Toshio Kuratomi, I raised it on the mailing list at:

https://code.activestate.com/lists/python-ideas/55992/

--

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Toshio Kuratomi]

Toshio Kuratomi  added the comment:

Additionally, the os module is supposed to closely follow the behaviour of the 
underlying operating system functions: 
https://docs.python.org/3/library/os.html  

> The design of all built-in operating system dependent modules of Python is 
> such that as long as the same functionality is available, it uses the same 
> interface; [..]

The POSIX symlimk function on which this is based has made the decision not to 
overwrite an existing symlink (See the EEXIST error in 
https://pubs.opengroup.org/onlinepubs/009695399/functions/symlink.html or man 
pages on symlink from one of the Linux distros: 
http://man7.org/linux/man-pages/man2/symlink.2.html )   As with many other 
POSIX-derived filesystem functions, the technique you propose, relying on 
atomic filesystem renames) would seem to be the standard method of writing 
race-resistant code.  Due to the mandate for the os module, it feels like that 
belongs in a utility function in custom code or another module rather than 
something for the os module.

A couple of thoughts on what you could do instead:

* A collection of utility functions that fixed race-conditions in filesystem 
handling could make a nice third party module on pypi.

* The stdlib shutil module provides an API that's supposed to be easier to 
implement common use cases than the os.* functions.  Perhaps you could propose 
your idea to the python-ideas mailing list as a new function in that module and 
see what people think of that?

--
nosy: +a.badger

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

So what? Detected problem is better than non-detected problem. If and 
unexpected exception causes troubles in your code, it is up to you what to do 
with it: silence it, terminate an application, try to recreate a symlink in 
other place, etc. In any case this will not solve bigger problem that you have: 
attacker is able to change your symlinks.

--

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Tom Hale]

Tom Hale  added the comment:

Yes, but by default (because of difficulty) people won't check for this case:

1. I delete existing symlink in order to recreate it
2. Attacker watching symlink finds it deleted and recreates it
3. I try to create symlink, and an unexpected exception is raised

--

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Serhiy Storchaka]

Serhiy Storchaka  added the comment:

If the symlink can be recreated, it can also be changed after creation.

--
nosy: +serhiy.storchaka

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Tom Hale]

Tom Hale  added the comment:

The most correct work-around I believe exists is:

(updates at: https://stackoverflow.com/a/55742015/5353461)

def symlink_force(target, link_name):
'''
Create a symbolic link pointing to target named link_name.
Overwrite target if it exists.
'''

# os.replace may fail if files are on different filesystems.
# Therefore, use the directory of target
link_dir = os.path.dirname(target)

# os.symlink requires that the target does NOT exist.
# Avoid race condition of file creation between mktemp and symlink:
while True:
temp_pathname = tempfile.mktemp(suffix='.tmp', \
prefix='symlink_force_tmp-', dir=link_dir)
try:
os.symlink(target, temp_pathname)
break  # Success, exit loop
except FileExistsError:
time.sleep(0.001)  # Prevent high load in pathological 
conditions
except:
raise
os.replace(temp_pathname, link_name)

An unlikely race condition still remains: the symlink created at the 
randomly-named `temp_path` could be modified between creation and 
rename/replacing the specified link name.

Suggestions for improvement welcome.

--
type:  -> security

___
Python tracker 

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

[issue36656] Allow os.symlink(src, target, force=True) to prevent race conditions

[Tom Hale]

New submission from Tom Hale :

I cannot find a race-condition-free way to force overwrite an existing symlink.

os.symlink() requires that the target does not exist, meaning that it could be 
created via race condition the two workaround solutions that I've seen:

1. Unlink existing symlink (could be recreated, causing following symlink to 
fail)

2. Create a new temporary symlink, then overwrite target (temp could be changed 
between creation and replace.

The additional gotcha with the safer (because the attack filename is unknown) 
option (2) is that replace() may fail if the two files are on separate 
filesystems.

I suggest an additional `force=` argument to os.symlink(), defaulting to 
`False` for backward compatibility, but allowing atomic overwriting of a 
symlink when set to `True`.

I would be willing to look into a PR for this.

Prior art:  https://stackoverflow.com/a/55742015/5353461

--
messages: 340474
nosy: Tom Hale
priority: normal
severity: normal
status: open
title: Allow os.symlink(src, target, force=True) to prevent race conditions
versions: Python 3.7

___
Python tracker 

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