diff options
author | Simon Kelley <simon@thekelleys.org.uk> | 2012-09-02 13:29:51 +0100 |
---|---|---|
committer | Simon Kelley <simon@thekelleys.org.uk> | 2012-09-02 13:29:51 +0100 |
commit | 79cfefd8561791904709ed60ae0f93e71f3dd8c6 (patch) | |
tree | fb732aecc8c59d4b1222a455bd0c120b131c97b3 | |
parent | 0c0d4793acb55a4789d5ebe88a2bd36c9397d48e (diff) | |
download | dnsmasq-79cfefd8561791904709ed60ae0f93e71f3dd8c6.tar.gz |
Make pid-file creation immune to symlink attack.
-rw-r--r-- | src/dnsmasq.c | 45 |
1 files changed, 39 insertions, 6 deletions
diff --git a/src/dnsmasq.c b/src/dnsmasq.c index 6abf517..6abd201 100644 --- a/src/dnsmasq.c +++ b/src/dnsmasq.c @@ -383,15 +383,48 @@ int main (int argc, char **argv) /* write pidfile _after_ forking ! */ if (daemon->runfile) { - FILE *pidfile; + int fd, err = 0; + + sprintf(daemon->namebuff, "%d\n", (int) getpid()); + + /* Explanation: Some installations of dnsmasq (eg Debian/Ubuntu) locate the pid-file + in a directory which is writable by the non-privileged user that dnsmasq runs as. This + allows the daemon to delete the file as part of its shutdown. This is a security hole to the + extent that an attacker running as the unprivileged user could replace the pidfile with a + symlink, and have the target of that symlink overwritten as root next time dnsmasq starts. + + The folowing code first deletes any existing file, and then opens it with the O_EXCL flag, + ensuring that the open() fails should there be any existing file (because the unlink() failed, + or an attacker exploited the race between unlink() and open()). This ensures that no symlink + attack can succeed. + + Any compromise of the non-privileged user still theoretically allows the pid-file to be + replaced whilst dnsmasq is running. The worst that could allow is that the usual + "shutdown dnsmasq" shell command could be tricked into stopping any other process. + + Note that if dnsmasq is started as non-root (eg for testing) it silently ignores + failure to write the pid-file. + */ + + unlink(daemon->runfile); - /* only complain if started as root */ - if ((pidfile = fopen(daemon->runfile, "w"))) + if ((fd = open(daemon->runfile, O_WRONLY|O_CREAT|O_TRUNC|O_EXCL, S_IWUSR|S_IRUSR|S_IRGRP|S_IROTH)) == -1) { - fprintf(pidfile, "%d\n", (int) getpid()); - fclose(pidfile); + /* only complain if started as root */ + if (getuid() == 0) + err = 1; } - else if (getuid() == 0) + else + { + if (!read_write(fd, (unsigned char *)daemon->namebuff, strlen(daemon->namebuff), 0)) + err = 1; + + while (!err && close(fd) == -1) + if (!retry_send()) + err = 1; + } + + if (err) { send_event(err_pipe[1], EVENT_PIDFILE, errno, daemon->runfile); _exit(0); |