Description

When MoinMoin generates a file (for example a user config file, a trail file, etc.), it typically first creates the file, then sets the file permissions later. This means that files could have too relaxed file permissions in the (very very short) time in between, which is a potential (although very minor) security risk.

Example code from User.setBookmark:

•    1     def setBookmark(self, tm=None):
     2         """
     3         Set bookmark timestamp.
     4         
     5         @param tm: time (UTC UNIX timestamp), default: current time
     6         """
     7         if self.valid:
     8             if tm is None:
     9                 tm = time.time()
    10             bmfile = open(self.__filename() + ".bookmark", "w")
    11             bmfile.write(str(tm)+"\n")
    12             bmfile.close()
    13             try:
    14                 os.chmod(self.__filename() + ".bookmark", 0666 & config.umask)
    15             except OSError:
    16                 pass
  

The file is created first, and the permissions are only set later. Similar code occurs in many places (grep for chmod).

I am also not sure about the silent try/except. I think a problem with the chmod should at least be logged somehow, except of course on platforms where os.chmod is not available.

Steps to reproduce

Exploiting this relies on race conditions and should be very very difficult to do. One way to showcase the problem might be:

1. Add a delay (time.sleep) after closing the file in the code above to artificially generate some time for other processes to interfere.

2. Set the Wiki umask to a setting like 0744.

3. Log in as some user and set the recent changes bookmark to run the code above.
4. During the delay, log in to the machine running the wiki as some user.

5. cat filename, where filename is the name of the bookmark file. (Normally, one would not know this, but that does not solve other instances of this problem and relies on "security by obscurity", which is bad.)

Details

This wiki.

Workaround

Make sure that the wiki directory has the executable flag cleared for other users.

Discussion

This is a very minor issue because for the default umask of 0770, the main wiki directory will be completely inaccessible for other anyway, and group will be able to read and write everything anyway. The problem only exists if for some reason you want a umask like 0710. Also, web servers are mostly configured with very restrictive environment umasks. Still, I think it is desirable to be very conservative with file permissions.

Possible ideas for fixing the umask problem:

• I think that calling os.umask(0777 ^ config_umask) at the beginning of the MoinMoin invocation would be completely sufficient and eliminate the need for any os.chmod calls. This would automatically use umask config_umask & 0666 when regular files are created and umask config_umask when directories are created. (Python's open never seems to set the executable flag on newly generated files, so manually masking that out does not seem to be necessary. I have not seen this behaviour documented and do not know if this is Python-specific or C standard.)

• If there are any cases where a narrower umask is required, file generation should be wrapped by two calls to os.umask, one setting the more restrictive value and one restoring the old value. But I am not sure if any such code actually exists. If it does occur in many places, this should likely be factored out, e.g. into util.filesys.

• If fixing this requires too many changes or is not desirable for some other reason, the current behaviour should be documented at the place where the umask config setting is explained. This documentation should suggest avoiding umasks which have the executable flag set and readable flag cleared for group or other.

-- MalteHelmert 2005-02-11 20:15:16

Plan

• Priority:

• Assigned to: ThomasWaldmann

• Status: will be done in 1.5.5 - this is primarily a performance optimization and simplifies the code

CategoryMoinMoinBugFixed