[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

Bug#700280: marked as done (tpu: dnsmasq: chowning pid directory and writing there as root may lead to security issue)

Your message dated Tue, 12 Feb 2013 23:06:04 +0000
with message-id <1360710364.31703.13.camel@jacala.jungle.funky-badger.org>
and subject line Re: Bug#700280: tpu: dnsmasq: chowning pid directory and writing there as root may lead to security issue
has caused the Debian Bug report #700280,
regarding tpu: dnsmasq: chowning pid directory and writing there as root may lead to security issue
to be marked as done.

This means that you claim that the problem has been dealt with.
If this is not the case it is now your responsibility to reopen the
Bug report if necessary, and/or fix the problem forthwith.

(NB: If you are a system administrator and have no idea what this
message is talking about, this may indicate a serious mail system
misconfiguration somewhere. Please contact owner@bugs.debian.org
immediately.)


-- 
700280: http://bugs.debian.org/cgi-bin/bugreport.cgi?bug=700280
Debian Bug Tracking System
Contact owner@bugs.debian.org with problems
--- Begin Message --- 
Package: release.debian.org
Severity: normal
User: release.debian.org@packages.debian.org
Usertags: tpu

Hi,

I prepared a tpu update for dnsmasq to isolate the fix for this
problem. It would not be appropriate for me to clear this for
upload myself; please could someone review and approve it.

Debdiff attached, versioned 2.62-3+deb70u1.
(Ignore the changelog.rej noise, it will not be in the upload.)

Maintainer: this is notice of intent to NMU this fix in to
wheezy, so I will delay by two days to give you chance to
object.

Thanks,

-- 
Jonathan Wiltshire                                      jmw@debian.org
Debian Developer                         http://people.debian.org/~jmw

4096R: 0xD3524C51 / 0A55 B7C5 1223 3942 86EC  74C3 5394 479D D352 4C51

<directhex> i have six years of solaris sysadmin experience, from
            8->10. i am well qualified to say it is made from bonghits
			layered on top of bonghits

diff -u dnsmasq-2.62/debian/changelog dnsmasq-2.62/debian/changelog
--- dnsmasq-2.62/debian/changelog
+++ dnsmasq-2.62/debian/changelog
@@ -1,3 +1,11 @@
+dnsmasq (2.62-3+deb70u1) testing-proposed-updates; urgency=low
+
+  * Non-maintainer upload.
+  * Backport fix for symlink attacks from 2.63-4
+    (Closes: #686484)
+
+ -- Jonathan Wiltshire <jmw@debian.org>  Sun, 10 Feb 2013 21:07:17 +0000
+
 dnsmasq (2.62-3) unstable; urgency=low
 
    * Do resolvconf and /etc/default startup logic when
only in patch2:
unchanged:
--- dnsmasq-2.62.orig/src/dnsmasq.c
+++ dnsmasq-2.62/src/dnsmasq.c
@@ -371,15 +371,48 @@
       /* 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);
only in patch2:
unchanged:
--- dnsmasq-2.62.orig/debian/changelog.rej
+++ dnsmasq-2.62/debian/changelog.rej
@@ -0,0 +1,12 @@
+--- debian/changelog
++++ debian/changelog
+@@ -1,3 +1,9 @@
++dnsmasq (2.63-4) unstable; urgency=low
++
++   * Make pid-file creation immune to symlink attacks. (closes: #686484)
++	
++ -- Simon Kelley <simon@thekelleys.org.uk>  Fri, 21 Sep 2012 17:16:34 +0000
++
+ dnsmasq (2.63-3) unstable; urgency=low
+ 
+    * Move adduser dependency to dnsmasq-base. (closes: #686694)

Attachment: signature.asc
Description: Digital signature


--- End Message ---
--- Begin Message --- 
On Sun, 2013-02-10 at 22:26 +0000, Jonathan Wiltshire wrote:
> On Sun, Feb 10, 2013 at 10:19:07PM +0000, Adam D. Barratt wrote:
> > On Sun, 2013-02-10 at 21:37 +0000, Jonathan Wiltshire wrote:
> > > I prepared a tpu update for dnsmasq to isolate the fix for this
> > > problem. It would not be appropriate for me to clear this for
> > > upload myself; please could someone review and approve it.
> > > 
> > > Debdiff attached, versioned 2.62-3+deb70u1.
> > 
> > +deb7u1, please. :) Please go ahead.
> 
> Awww :( Uploaded, delayed 2.

Unblocked; thanks.

Regards,

Adam

--- End Message ---
Reply to: