Avoid race condition when asserting file permissions in Python

An application wants to parse and "execute" a file, and wants to assert the file is executable for security reasons.

A moments thought and you realize this initial code has a race condition that makes the security scheme ineffective:

import os

class ExecutionError (Exception):
    pass

def execute_file(filepath):
    """Execute serialized command inside @filepath

    The file must be executable (comparable to a shell script)
    >>> execute_file(__file__)  # doctest: +ELLIPSIS
    Traceback (most recent call last):
        ...
    ExecutionError: ... (not executable)
    """
    if not os.path.exists(filepath):
        raise IOError('"%s" does not exist' % (filepath, ))
    if not os.access(filepath, os.X_OK):
        raise ExecutionError('No permission to run "%s" (not executable)' %
                filepath)

    data = open(filepath).read()

    print '"Dummy execute"'
    print data

The race condition exists between

os.access(filepath, os.X_OK)

and

data = open(filepath).read()

Since there is a possibility of the file being overwritten with a non-executable file of different content between these two system calls.

The first solution I have is to change the order of the critical calls (and skip the now-redundant existance check):

fobj = open(filepath, "rb")
if not os.access(filepath, os.X_OK):
    raise ExecutionError('No permission to run "%s" (not executable)' %
            filepath)

data = fobj.read()

Does this solve the race condition? How can I solve it properly?

Security scheme rationale, briefly (I thought)

The file will be able to carry out arbitrary commands inside its environment, so it is comparable to a shell script.

There was a security hole on free desktops with .desktop files that define applications: The file may specify any executable with arguments, and it may choose its own icon and name. So a randomly downloaded file could hide behind any name or icon and do anything. That was bad.

This was solved b开发者_运维问答y requiring that .desktop files have the executable bit set, otherwise they will not be rendered with name/icon, and the free desktop will ask the user if it wants to start the program before commencing.

Compare this to Mac OS X's very good design: "This program has been downloaded from the web, are you sure you want to open it?".

So in allegory with this, and the fact that you have to chmod +x shell scripts that you download, I thought about the design in the question above.

Closing words

Maybe in conclusion, maybe we should keep it simple: If the file must be executable, make it executable and let the kernel execute it when invoked by the user. Delegation of the task to where it belongs.

The executability is attached to the file you open, there is nothing stopping several files from pointing to the inode containing the data you wish to read. In other words, the same data may be readable from a non-executable file elsewhere in the same filesystem. Furthermore, even after opening the file, you can't prevent the executability of that same file from changing, it could even be unlinked.

The "best effort" available to you as I see it would be do checks using os.fstat on the opened file, and check protection mode and modification time before and after, but at best this will only reduce the possibility that changes go undetected while you read the file.

On second thoughts, if you're the original creator of the data in this file, you could consider writing an inode that's never linked to the filesystem in the first place, this a common technique in memory sharing via files. Alternatively if the data contained must eventually made public to other users, you could use file locking, and then progressively extend the protection bits to those users that require it.

Ultimately you must ensure malicious users simply don't have write access to the file.

You cannot entirely solve this race condition -- e.g., in the version where you first open, then check permissions, it's possible that the permissions get changed just after you've opened the file and just before you've changed the permissions.

If you can atomically move the file to a directory where the potential bad guys can't reach, then you can rest assured that nothing about the file will be changed from under your nose while you're dealing with it. If the potential bad guys can reach everywhere, or you can't move the file to where they can't reach, there's no defense.

BTW, it's not clear to me how this scheme, even if it could be made to work, would actually add any security -- surely if the bad guys can put poisoned content in the file it's not beyond them to chmod +x it as well?

The best you can do is :

• save the permission.
• change it to your own unique user (something with the program name) and forbid others to run it.
• make you checks (on the saved permission if needed).
• run your process.
• set back the permission to the saved ones.

Of course, there are drawbacks, but if your use case is as simple as you seem to say, it could do the trick.

You should change the files ownership such that an attacker cannot access it "chown root:root file_name". Do a "chmod 700 file_name" so that no other accounts can read/write/execute the file. This avoids the problem of a TOCTOU all together and this is how people prevent files from being modified by an attacker who has a user account on your system.

Another way to do it is to change the file name to something unexpected, or even copy the entire file - if it's not too big - to a temp dir (encrypted if necessary), make you checks, then rename / copy the file back.

Of course it's a very heavy process.

But you end up with that because the system has not been set for safety from the beginning. A safe program would sign or encrypt data he wants to keep safe. In your case, it's not possible.

Unless you realy on heavy encrypting, there is not way to ensure 100 safety on a machine you don't control.