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

Bug#58344: marked as done ([PATCH] /tmp race in asp)



Your message dated Thu, 2 Aug 2001 14:16:58 +0100
with message-id <20010802141658.A12397@flatline.org.uk>
and subject line asp bugs fixed by QA upload
has caused the attached Bug report 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 I am
talking about this indicates a serious mail system misconfiguration
somewhere.  Please contact me immediately.)

Darren Benham
(administrator, Debian Bugs database)

--------------------------------------
Received: (at submit) by bugs.debian.org; 17 Feb 2000 22:54:14 +0000
Received: (qmail 12422 invoked from network); 17 Feb 2000 22:54:13 -0000
Received: from navy.csi.cam.ac.uk (exim@131.111.8.49)
  by master.debian.org with SMTP; 17 Feb 2000 22:54:13 -0000
Received: from crp22.trin.cam.ac.uk ([131.111.193.222] ident=mail)
	by navy.csi.cam.ac.uk with esmtp (Exim 3.13 #1)
	id 12LZoE-000466-00; Thu, 17 Feb 2000 22:54:06 +0000
Received: from cph by crp22.trin.cam.ac.uk with local (Exim 3.12 #1 (Debian))
	id 12LZoE-0003gG-00; Thu, 17 Feb 2000 22:54:06 +0000
From: Colin Phipps <crp22@cam.ac.uk>
To: Debian Bug Tracking System <submit@bugs.debian.org>
Subject: [PATCH] /tmp race in asp
X-Reportbug-Version: 0.50
X-Mailer: reportbug 0.50
Date: Thu, 17 Feb 2000 22:54:06 +0000
Message-Id: <E12LZoE-0003gG-00@crp22.trin.cam.ac.uk>

Package: asp
Version: 1.7
Severity: normal

asp has a feature (-u option) where it updates the /etc/hosts file with the
changed ip's of machines. The new hosts file is written to a temporary file
in /tmp. 

There is a race in the opening of this file: a malicious local user who 
guessed the filename could insert a file between the tmpnam(3) and fopen(3) 
calls (see the page in the libc docs for the standard warnings about using 
tmpnam and co). This would allow the user to modify the new hosts file 
befoer it was copied back to /etc. Obviously if they suceeded this would 
be a major security problem. 

Patch follows:

--- asp.c.orig	Thu Feb 17 22:23:47 2000
+++ asp.c	Thu Feb 17 22:33:33 2000
@@ -227,13 +227,17 @@ int match(char* buf, char* host){
 /* update_hosts trova la linea host in /etc/hosts e cambia il relativo
    indirizzo. Se addr e` NULL la linea viene eliminata. */
 void update_hosts(char* host, char* addr){
-  char buf[BUFSIZ+1], *name;
+  char buf[BUFSIZ+1], *name = HOSTS_PATH ".asp";
   FILE *fp1, *fp2;
   int c, m;
 
   if((fp1 = fopen(HOSTS_PATH, "r+")) == NULL) asp_perror("fopen");
 
-  if((name = tmpnam(NULL)) == NULL) asp_perror("tmpnam");
+  /* CPhipps 2000/02/17 - Use temporary file in /etc, so we can 
+   * rename(2) it to /etc/hosts cleanly once we're done.
+   * I'm assuming there will only ever be one copy of asp running, if 
+   * that's not true than this open should be O_EXCL and we need stale 
+   * temp file handling too. But I doubt it's needed. */
   if((fp2 = fopen(name, "w")) == NULL) asp_perror("fopen");
 
   do{ 
@@ -246,7 +250,19 @@ void update_hosts(char* host, char* addr
     if(addr != NULL) fprintf(fp2, "%s\t%s\n", addr, host);
     while((c = fgetc(fp1)) != EOF)
       fputc(c, fp2);
-    if(remove(HOSTS_PATH) == -1) asp_perror("remove");
+    /* CPhipps 2000/02/17 - quoting from the libc docs:
+
+ One useful feature of `rename' is that the meaning of the name
+ NEWNAME changes "atomically" from any previously existing file by
+ that name to its new meaning (the file that was called OLDNAME).
+ There is no instant at which NEWNAME is nonexistent "in between"
+ the old meaning and the new meaning.  If there is a system crash
+ during the operation, it is possible for both names to still
+ exist; but NEWNAME will always be intact if it exists at all.
+
+     * /etc/hosts is an important system file.. we want it 
+     * to always exist, so we use this feature */
+                                   
     if(rename(name, HOSTS_PATH) == -1) asp_perror("rename");
   }else{
     if(addr != NULL)

I took the opportunity to modify the file handling to use rename(2) to
overwrite the old hosts file. This way, there is no gap between removing
/etc/hosts and replacing it.. which is a good thing for an important system
file.

Here is an strace fragment from before the patch, to show the problem:

open("/etc/hosts", O_RDWR)              = 5
stat("/tmp", {st_mode=S_IFDIR|S_ISVTX|0777, st_size=55296, ...}) = 0
gettimeofday({950826938, 673167}, NULL) = 0
getpid()                                = 13144
stat("/tmp/fileCrKTkX", 0xbfffdb1c)     = -1 ENOENT (No such file or directory)
open("/tmp/fileCrKTkX", O_WRONLY|O_CREAT|O_TRUNC, 0666) = 6

Colin

-- System Information
Debian Release: woody
Architecture: i386
Kernel: Linux crp22 2.2.15pre7-cph3 #1 Sat Feb 12 22:27:57 GMT 2000 i686

Versions of packages asp depends on:
ii  libc6                         2.1.3-2    GNU C Library: Shared libraries an

---------------------------------------
Received: (at 58344-done) by bugs.debian.org; 2 Aug 2001 13:17:19 +0000
>From cjw44@flatline.org.uk Thu Aug 02 08:17:19 2001
Return-path: <cjw44@flatline.org.uk>
Received: from (mccoy.flatline.org.uk) [::ffff:195.8.181.200] 
	by master.debian.org with esmtp (Exim 3.12 1 (Debian))
	id 15SILh-0002PE-00; Thu, 02 Aug 2001 08:17:13 -0500
Received: from cjw44 by mccoy.flatline.org.uk with local (Exim 3.12 #1 (Debian))
	id 15SILS-0003G4-00; Thu, 02 Aug 2001 14:16:58 +0100
Date: Thu, 2 Aug 2001 14:16:58 +0100
From: Colin Watson <cjwatson@flatline.org.uk>
To: 50101-done@bugs.debian.org, 58344-done@bugs.debian.org,
	70129-done@bugs.debian.org, 91111-done@bugs.debian.org,
	91368-done@bugs.debian.org, 91388-done@bugs.debian.org
Subject: asp bugs fixed by QA upload
Message-ID: <20010802141658.A12397@flatline.org.uk>
Mime-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
User-Agent: Mutt/1.2.5i
Sender: Colin Watson <cjw44@flatline.org.uk>
Delivered-To: 58344-done@bugs.debian.org

These bugs were all fixed in a QA Group upload by Gergely Nagy. As such,
I'm closing them now.

asp (1.8-1) unstable; urgency=low

  * Non-maintainer upload, set maintainer to QA group
  * New upstream release, containing tmp race fix (Closes: Bug#58344)
  * Builds without requiring user input (Closes: Bug#50101, Bug#70129)
  * FHS Compliance (Closes: Bug#91111, Bug#91368, Bug#91388)
  * Bumped Standards-version to 3.5.2
  * Converted to debhelper

 -- Gergely Nagy <8@free.bsd.hu>  Fri, 13 Apr 2001 18:06:53 +0200

Cheers,

-- 
Colin Watson                                  [cjwatson@flatline.org.uk]



Reply to: