Hi,
Simon Kelley wrote:
> dnsmasq (2.63-4) unstable; urgency=low
> .
> * Make pid-file creation immune to symlink attacks. (closes: #686484)
How about this patch to fix the same in squeeze?
Thanks,
Jonathan
---
debian/changelog | 7 +++++++
src/dnsmasq.c | 45 ++++++++++++++++++++++++++++++++++++++-------
2 files changed, 45 insertions(+), 7 deletions(-)
diff --git a/debian/changelog b/debian/changelog
index 3955c750..23cfa0da 100644
--- a/debian/changelog
+++ b/debian/changelog
@@ -1,3 +1,10 @@
+dnsmasq (2.55-2+squeeze0.1) stable; urgency=low
+
+ [ Simon Kelley ]
+ * Make pid-file creation immune to symlink attacks. (closes: #686484)
+
+ -- Jonathan Nieder <jrnie...@gmail.com> Mon, 22 Oct 2012 00:21:24 -0700
+
dnsmasq (2.55-2) unstable; urgency=high
* Fix crash on double free. (closes: #597205)
diff --git a/src/dnsmasq.c b/src/dnsmasq.c
index 727d5e2e..d607e26d 100644
--- a/src/dnsmasq.c
+++ b/src/dnsmasq.c
@@ -327,16 +327,47 @@ 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 (!read_write(fd, (unsigned char *)daemon->namebuff,
strlen(daemon->namebuff), 0))
+ err = 1;
+
+ while (!err && close(fd) == -1)
+ if (!retry_send())
+ err = 1;
}
- else if (getuid() == 0)
- {
+
+ if (err)
send_event(err_pipe[1], EVENT_PIDFILE, errno);
_exit(0);
}
--
1.8.0
--
To UNSUBSCRIBE, email to debian-bugs-rc-requ...@lists.debian.org
with a subject of "unsubscribe". Trouble? Contact listmas...@lists.debian.org
