[issue36021] [Security][Windows] webbrowser: WindowsDefault uses os.startfile() and so can be abused to run arbitrary commands

[Eryk Sun]

Eryk Sun <eryksun at gmail.com> added the comment:

> os.access(path, os.X_OK) is specific to Unix. It doesn't make sense 
> on Windows. 

It doesn't make sense with the current implementation of os.access, and not as Stéphane used it. Even if we completely implemented os.access, the problem is that most files grant execute access to the owner, "Users", or "Authenticated Users". Typically we have to override inheritance to prevent granting execute access, or add an entry that denies execute access.

However, it's not that it makes no sense in general. CreateProcess does require execute access on a file. This includes both DACL discretionary access and SACL mandatory access. ShellExecuteEx ultimately calls CreateProcess if it's running a "file:" URL, so execute access certainly matters in this case. For example, I've denied execute access on the following file:

    >>> os.startfile('file:///C:/Temp/test/test.exe')
    Traceback (most recent call last):
      File "<stdin>", line 1, in <module>
    PermissionError: [WinError 5] Access is denied: 'file:///C:/Temp/test/test.exe'

On the other hand, if we're talking about data files and scripts, ShellExecute[Ex] doesn't check for execute access because it's generally used to "open" files. It wouldn't be a security barrier, anyway. It's easy enough for a program to call AssocQueryString to get the command-line template for a protocol or file type, manually replace template parameters, and execute the command directly via CreateProcess. 

> os.access() is implemented with GetFileAttributesW() on Windows. The
> mode argument is more or less ignored.

The readonly file attribute denies W_OK access, similar to how the [i]mmutable file attribute works in some Unix systems (e.g. Linux lsattr and chattr +i).

----------
nosy: +eryksun

_______________________________________
Python tracker <report at bugs.python.org>
<https://bugs.python.org/issue36021>
_______________________________________