views:

259

answers:

5

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

+2  A: 

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?

Alex Martelli
+3  A: 

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.

Matt Joiner
@Michael Brooks: I'm not sure either why the OP is interested in the executability of the file by the current user. Of far more interest is the _writability_. Perhaps their intention is to read a file if it's executable, and then execute it, I'm not sure...
Matt Joiner
I've racked my brain for improvements to this, they're simply not there. A "copy on write" hard link or copy operation is the highest level way I can think of to ensure the race condition can't occur, but these don't exist, afaik.
Matt Joiner
Thanks for the thought effort spent! I think I see what you are saying, and the security model can't stand on the execute access on its own, write access must either be restricted or the user must trust the file.
kaizer.se
Under most (all that I know of) unix file systems, the permissions are attached to the inode, not to the directory entry, so it would be hard for it to have some permissions when accessed via one path and other permissions via another path.
Vatine
A: 

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.

e-satis
Sounds reasonable.
Matt Joiner
For a non-intrusive application (not having its own user set up etc), that's really to complicated just to make sure the user knows what it is doing.
kaizer.se
A: 

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.

Rook
A: 

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.

e-satis
It is for the safety of the user. If the file is not executable, it was not consciously installed. So the question is mostly about making sure this check is done in the right way -- there is no way to embed a signature in the file, one user's constructive command is another's destructive. Compare to a shell script.
kaizer.se