We're almost done now, except for the XXX marker in the code above. Before invoking the external program, there's one thing that needs to be done: closing all open files.
For instance, crontab has probably called a function such as getpwuid to obtain the login name of the calling user. Unless it also called endpwent(), the process will have two open files /etc/passwd and /etc/shadow, both opened for reading. When running the external command, it will inherit these open files. An open file handle for /etc/passwd is harmless, but one for /etc/shadow isn't. Remember, the user can specify any program he likes via the EDITOR variable, so why not write a small program that tries all file descriptors from, say 0 to 1024 in turn and try to read from them? If the user did that, lo and behold, he'd be able to display the contents of the /etc/shadow database containing the encrypted user passwords!
Therefore, it is important to be careful with open files when running an untrusted external program! Of course, crontab could probably avoid this particular problem by calling endpwent after it has looked up the user's name. However, getpwnam is probably not be the only function leaving open files lying around, so it is usually a good idea to close all unneeded open files.
Usually, it's best to close all open file descriptors except the standard ones (standard input, output and error) attached to descriptors 0, 1 and 2, respectively. The following code tries to do that in a portable way:
int fd;
#ifdef OPEN_MAX
fd = OPEN_MAX;
#elif defined(NOFILE)
fd = NOFILE;
#else
fd = getdtablesize();
#endif
while (fd-- > 2)
close(fd);
There is also a kernel feature worth mentioning in this context, which is the close-on-exec flag. This is not really interesting for people writing set-user applications, but for people writing library functions such as the getpw* class of functions that want to keep a file open across function calls. You can help applications writers that forget to close all open files (shame on them) and improve the security of their programs by setting the close-on-exec flag on the file descriptors you use.
The close-on-exec flag tells the kernel to close the file when executing another program. The purpose of this flag is exactly to prevent the kind of information leak described above. Assume you've opened a file using fopen, then you can set the flag like this:
void set_cloexec(FILE *fp)
{
fcntl(fileno(fp), F_SETFD, FD_CLOEXEC);
}