Re: world writeable files

[Havoc Pennington <hp redhat com>]

"Colm Smyth [RU-Ireland]" <colms ireland sun com> writes:
> I'd suggest removing the call to umask altogether; it's hard for apps
> to do the right thing all the time, that's why it's generally set
> from the shell.
> 
> If there's no way to tell xmlSaveFile() to use the "ambient" umask,
> better pass the current value of umask(), obtained using something
> like:
> 
> 	umask(cmask = umask(0)); /* cmask contains the current umask
> 	*/

xmlSaveFile() does use the ambient mask.

Back in the mists of time, I was trying to provide some way to
configure the permissions on files created by the XML backend, and the
way it works is that it picks the permissions of the parent directory
(minus search bit). So this wasn't being done for the files created by
xmlSaveFile, the umask was used instead, and I had the umask(0) which
was one of the first lines of code in gconfd, copied from Steven's
daemon boilerplate in APUE. Result was 644 files. ;-) Oops.

Anyhow, at the moment umask(022) is theoretically having no effect
since the XML backend explicitly sets permissions, but you're probably
right it should come out.

There's a small race condition where we xmlSaveFile to a temporary
file, then chmod() the file, then rename it to its final location; the
umask gets used for the initial xmlSaveFile. An attacker could in
principle get access to the pre-chmod() file before we get a chance to
chmod() it. So that might be an argument for setting the umask to 066,
if we're going to keep the idea of configuring perms from the parent
directory. (i.e. we want to xmlSaveFile as completely private, and
then move it to the perhaps more open permissions.)

As a better solution, we could just write a util function
xmlSaveFileWithPerms which temporarily changes the umask around the
xmlSaveFile then puts it back, and calls chmod().

Havoc