[PATCH] tracd pidfile is created world-writable

[olly@…]

If tracd is run with -d and --pidfile, then while daemonising the umask is set to 0 and the pidfile is then created with file permissions 0777. This means that any user on the system can change its contents, which could result in a different process being killed if someone does kill -INT `cat pidfile`.

[cboos]

Seems worth fixing.

[Remy Blank <remy.blank@…>]

The problem is even worse: due to the os.umask(0) call in trac.util.daemon.py, any files created by Trac will have 0777 permissions. I don't think this was intended.

I see two possible solutions:

• Drop the os.umask(0) call and keep the calling umask. The UNIX programming FAQ, section 1.7, point 5 lists this step as optional, to "have complete control over the permissions of anything we write". For tracd, it might make more sense to use the calling umask so that it can be customized by the caller.

trac/util/daemon.py

@@ -50 +50 @@
 
     # Decouple from parent environment
     os.chdir('/')
-    os.umask(0)
     os.setsid()
 
     # Perform second fork

• Add a command-line option to pass the desired umask (with a default of e.g. 0644).

Please let me know if the second solution is preferred, and I'll provide a patch.

[Remy Blank <remy.blank@…>]

A related issue is that files created by Trac using os.open() (attachments, plugins installed by the admin interface) get a default mode of 0777. This means that they are marked as executable. Whereas files created with open() get a default mode of 0666.

This also affects other frontends (CGI, mod_python).

I would suggest passing a 0666 mode to every call to os.open(). There are only two occurrences in 0.11-stable, one in trac/util/__init__.py and one in trac/admin/web_ui.py.

Let me know if you would like me to submit a patch.

[cboos]

See also similar issue Django:ticket:6994.

They now use a default value of 022 for the umask and have it configurable. I'd say for Trac, changing the default would be enough.

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable [7346] making umask configurable, with 022 as default

[Remy Blank <remy.blank@…>]

Patch against 0.11-stable [7346] setting mode of created files to 0666 instead of 0777

[cboos]

Patches applied in r7358 and r7359. Thanks!