6750420 2001-07-13 08:08 +0300  /76 rader/ Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
Sänt av: joel@lysator.liu.se
Importerad: 2001-07-16  07:55  av Brevbäraren
Extern mottagare: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
Extern kopiemottagare: Charles Stevenson <core@ezlink.com>
Extern kopiemottagare: Bugtraq <bugtraq@securityfocus.com>
Extern kopiemottagare: Vuln-dev <vuln-dev@securityfocus.com>
Mottagare: Bugtraq (import) <17971>
Kommentar till text 6750452 av Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
    Sänt:     2001-07-16 08:11
Ärende: Re: Tripwire temporary files
------------------------------------------------------------
From: Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
To: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
Cc: Charles Stevenson <core@ezlink.com>,
 Bugtraq <bugtraq@securityfocus.com>,
 Vuln-dev <vuln-dev@securityfocus.com>
Message-ID: <20010713080836.A161232@messi.uku.fi>

On Thu, Jul 12, Cy Schubert - ITSD Open Systems Group wrote:

> I don't know whether the commercial version (2.4) has this bug (haven't 
> installed it yet, though as the free version is probably based on the 
> commercial version, I suspect (guess) it might be.

I have reported the tempfile issue to tripwire back in January. I was
under the impression that (then upcoming 2.4) would have this fixed.
I haven't checked if it fixes the bug, but AFAIK it has the
TEMPDIRECTORY config option so you can use 'safe' temp directory.

> And for Tripwire-2.3.1 the patch is:
> --- src/core/unix/unixfsservices.cpp.orig	Sat Feb 24 11:02:12 2001
> +++ src/core/unix/unixfsservices.cpp	Tue Jul 10 21:40:37 2001
> @@ -243,6 +243,7 @@
>  {
>      char* pchTempFileName;
>      char szTemplate[MAXPATHLEN];
> +    int fd;
>  
>  #ifdef _UNICODE
>      // convert template from wide character to multi-byte string
> @@ -253,13 +254,14 @@
>      strcpy( szTemplate, strName.c_str() );
>  #endif
>  
> -    // create temp filename
> -    pchTempFileName = mktemp( szTemplate );
> +    // create temp filename and check to see if mkstemp failed
> +   if ((fd = mkstemp( szTemplate )) == -1) {
> +     throw eFSServicesGeneric( strName );
> +   } else {
> +     close(fd);
> +   }
> +   pchTempFileName = szTemplate;
>  
> -    //check to see if mktemp failed
> -    if ( pchTempFileName == NULL || strlen(pchTempFileName) == 0) {
> -      throw eFSServicesGeneric( strName );
> -    }
>  
>      // change name so that it has the XXXXXX part filled in
>  #ifdef _UNICODE

If you look a little below you'll see a call to FileDelete(strName);
So first you create a file with mkstemp and then unlink it. And
because cFileArchive::OpenReadWrite(line 708) then opens the same
file(name) without O_EXCL there still is a race. So I don't think
this is a sufficient fix.  You should make
cFileArchive::OpenReadWrite use O_EXCL.  I have --> untested <--
patch (probably fails horribly ;-) for this:
http://www.uku.fi/~jhuuskon/Patches/tripwire-2.3.1-2-O_EXCL.patch

> We haven't had a chance to install the commercial version yet, however 
> if the commercial version is vulnerable (I've notified TripwireSecurity 
> of the possibility and I'm betting dollars to donuts that is might be) 
> a possible workaround would be to create a shared library with a 
> function named mktemp which would call mkstemp() as in the patches 
> above, then execute tripwire using LD_PRELOAD to load the mktemp 
> wrapper.

Back in january the binary tripwire 2.2.1 for linux was statically
compiled / linked. Can you use LD_PRELOAD with static executables ?

-Jarno

-- 
Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
(6750420) /Jarno Huuskonen <Jarno.Huuskonen@uku.fi>/(Ombruten)
Kommentar i text 6756119 av Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
6756119 2001-07-16 15:50 -0700  /46 rader/ Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
Sänt av: joel@lysator.liu.se
Importerad: 2001-07-17  06:53  av Brevbäraren
Extern mottagare: Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
Extern kopiemottagare: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
Extern kopiemottagare: Charles Stevenson <core@ezlink.com>
Extern kopiemottagare: Bugtraq <bugtraq@securityfocus.com>
Extern kopiemottagare: Vuln-dev <vuln-dev@securityfocus.com>
Externa svar till: Cy.Schubert@uumail.gov.bc.ca
Mottagare: Bugtraq (import) <18021>
Kommentar till text 6750420 av Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
Ärende: Re: Tripwire temporary files
------------------------------------------------------------
From: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
To: "Jarno Huuskonen" <Jarno.Huuskonen@uku.fi>
Cc: "Cy Schubert - ITSD Open Systems Group" <Cy.Schubert@uumail.gov.bc.ca>,
 "Charles Stevenson" <core@ezlink.com>,
 "Bugtraq" <bugtraq@securityfocus.com>,
 "Vuln-dev" <vuln-dev@securityfocus.com>
Message-ID: <200107162251.f6GMpNd15685@cwsys.cwsent.com>

In message <20010713080836.A161232@messi.uku.fi>, "Jarno Huuskonen" 
writes:
> If you look a little below you'll see a call to FileDelete(strName); So
> first you create a file with mkstemp and then unlink it. And because
> cFileArchive::OpenReadWrite(line 708) then opens the same file(name) without
> O_EXCL there still is a race. So I don't think this is a sufficient fix.
> You should make cFileArchive::OpenReadWrite use O_EXCL.
> I have --> untested <-- patch (probably fails horribly ;-) for this:
> http://www.uku.fi/~jhuuskon/Patches/tripwire-2.3.1-2-O_EXCL.patch

I applied your patch to the upcoming FreeBSD Tripwire-2.3.1 port.  I 
tested it  and it works!

> 
> > We haven't had a chance to install the commercial version yet, however 
> > if the commercial version is vulnerable (I've notified TripwireSecurity 
> > of the possibility and I'm betting dollars to donuts that is might be) 
> > a possible workaround would be to create a shared library with a 
> > function named mktemp which would call mkstemp() as in the patches 
> > above, then execute tripwire using LD_PRELOAD to load the mktemp 
> > wrapper.
> 
> On Thu, Jul 12, Cy Schubert - ITSD Open Systems Group wrote:
> Back in january the binary tripwire 2.2.1 for linux was statically
> compiled / linked. Can you use LD_PRELOAD with static executables ?

LD_PRELOAD only works on dynamically linked binaries.


Regards,                         Phone:  (250)387-8437
Cy Schubert                        Fax:  (250)387-5766
Team Leader, Sun/Alpha Team   Internet:  Cy.Schubert@osg.gov.bc.ca
Open Systems Group, ITSD, ISTA
Province of BC
(6756119) /Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>/
6750452 2001-07-12 18:56 -0700  /222 rader/ Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
Sänt av: joel@lysator.liu.se
Importerad: 2001-07-16  08:11  av Brevbäraren
Extern mottagare: Charles Stevenson <core@ezlink.com>
Extern kopiemottagare: Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
Extern kopiemottagare: Bugtraq <bugtraq@securityfocus.com>
Extern kopiemottagare: Vuln-dev <vuln-dev@securityfocus.com>
Externa svar till: Cy.Schubert@uumail.gov.bc.ca
Mottagare: Bugtraq (import) <17981>
Kommentar till text 6729559 av Charles Stevenson <core@ezlink.com>
Ärende: Re: Tripwire temporary files
------------------------------------------------------------
From: Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>
To: Charles Stevenson <core@ezlink.com>
Cc: Jarno Huuskonen <Jarno.Huuskonen@uku.fi>,
 Bugtraq <bugtraq@securityfocus.com>,
 Vuln-dev <vuln-dev@securityfocus.com>
Message-ID: <200107130156.f6D1uhL75338@cwsys.cwsent.com>

In message <3B4A936D.FF2DA075@ezlink.com>, Charles Stevenson writes:
> Jarno Huuskonen wrote:
> 
> >  After that I looked at the tripwire sources and confirmed the problem.
> >  (See e.g. core/archive.cpp, core/unix/unixfsservices.cpp and
> >  tw/textreportviewer.cpp).
> 
> If you noticed a few more lines down the file get's removed.
> 
> -> TSTRING& cUnixFSServices::MakeTempFilename( TSTRING& strName ) const
> throw(eFSServices)
> -> {
> -> ...
> ->     // create temp filename
> ->     pchTempFileName = mktemp( szTemplate );
> -> ...
> ->     strName = pchTempFileName;
> -> ...
> -> 
> ->     // Linux creates the file!!  Doh!
> ->     // So I'll always attempt to delete it -bam
> ->     FileDelete( strName );
> -> 
> -> 	return( strName );
> -> }
> 
> So it's going to be a really tight race since the file would have to be
> created just after FileDelete is called.
> 
> -> void cLockedTemporaryFileArchive::OpenReadWrite( const TCHAR*
> filename, uint32 openFlags )
> -> {
> -> ...
> ->     // if filename is NULL, create a temp file for the caller
> ->     if( filename == NULL )
> ->       {
> ->         try
> ->           {
> ->             iFSServices::GetInstance()->GetTempDirName( strTempFile
> );
> ->             strTempFile += _T("twtempXXXXXX");  
> ->             iFSServices::GetInstance()->MakeTempFilename( strTempFile
> );
> -> ...
> ->     // open file
> ->     mCurrentFilename = filename ? filename : strTempFile.c_str();
> ->     mCurrentFile.Open( mCurrentFilename, flags );
> -> ...
> -> }
> 
> I've been trying to think of a way to exploit this. The only way I could
> foresee was if you could run an exploit as a cron timed with a tripwire
> cron run as root and the exploit would create a lot of symlinks right
> before tripwire runs which could allow creation of files as root but if
> the file get's removed then really what you'd need is a way to watch all
> the symlinks you've created and the instant one is removed create it
> again (run on sentence;).  Any ideas?
> 
> The patch should be to use mkstemp() if the OS is Linux.

Here are patches to Tripwire-1.3.1 and -2.3.1-2.  The 1.3.1 patches
have been in use in the FreeBSD port for over a year, while the
Tripwire-2.3.1 patches have been in the upcoming FreeBSD
Tripwire-2.3.1  port which will be released once I've completed
testing Tripwire 2.3.1  in parallel with 1.3.1.

I don't know whether the commercial version (2.4) has this bug
(haven't  installed it yet, though as the free version is probably
based on the  commercial version, I suspect (guess) it might be.

Tripwire 1.3.1 patches follow:

--- src/config.parse.c.orig	Tue Jun 13 23:24:14 2000
+++ src/config.parse.c	Tue Jun 13 23:30:35 2000
@@ -55,7 +55,6 @@
 #endif
 
 /* prototypes */
-char *mktemp();
 static void configfile_descend();
 
 #ifndef L_tmpnam
@@ -105,8 +104,8 @@
     };
     (void) strcpy(tmpfilename, TEMPFILE_TEMPLATE);
 
-    if ((char *) mktemp(tmpfilename) == NULL) {
-	perror("configfile_read: mktemp()");
+    if (mkstemp(tmpfilename) == -1) {
+	perror("configfile_read: mkstemp()");
 	exit(1);
     }
 
--- src/dbase.build.c.orig	Tue May  4 17:31:00 1999
+++ src/dbase.build.c	Tue Jun 13 23:40:06 2000
@@ -60,7 +60,6 @@
 int files_scanned_num = 0;
 
 /* prototypes */
-char *mktemp();
 
 /* new database checking routines */
 static void 	database_record_write();
@@ -135,8 +134,8 @@
 	    die_with_err("malloc() failed in database_build", (char *) NULL);
 	(void) strcpy(tmpfilename, TEMPFILE_TEMPLATE);
 
-	if ((char *) mktemp(tmpfilename) == NULL)
-	    die_with_err("database_build: mktemp()", (char *) NULL);
+	if (mkstemp(tmpfilename) == -1)
+	    die_with_err("database_build: mkstemp()", (char *) NULL);
 
 	(void) strcpy(tempdatabase_file, tmpfilename);
 	(void) strcpy(database, tempdatabase_file);
@@ -814,8 +813,8 @@
     /* build temporary file name */
     (void) strcpy(backup_name, TEMPFILE_TEMPLATE);
 
-    if ((char *) mktemp(backup_name) == NULL) {
-	die_with_err("copy_database_to_backup: mktemp() failed!",
NULL);
+    if (mkstemp(backup_name) == -1) {
+	die_with_err("copy_database_to_backup: mkstemp() failed!", NULL);
     }
 
     strcpy (database_backupfile, backup_name);
--- src/siggen.c.orig	Tue Jun 13 23:42:53 2000
+++ src/siggen.c	Tue Jun 13 23:43:27 2000
@@ -52,7 +52,6 @@
 
 extern int optind;
 int debuglevel = 0;
-char *mktemp();
 
 int (*pf_signatures [NUM_SIGS]) () = {
 					SIG0FUNC,
@@ -172,8 +171,8 @@
 	};
 	(void) strcpy(tmpfilename, "/tmp/twzXXXXXX");
 
-	if ((char *) mktemp(tmpfilename) == NULL) {
-	    perror("siggen: mktemp()");
+	if (mkstemp(tmpfilename) == -1) {
+	    perror("siggen: mkstemp()");
 	    exit(1);
 	}
 
--- src/utils.c.orig	Tue Jun 13 23:43:01 2000
+++ src/utils.c	Tue Jun 13 23:43:50 2000
@@ -856,8 +856,8 @@
     int fd;
 
     (void) strcpy(tmp, TEMPFILE_TEMPLATE);
-    if ((char *) mktemp(tmp) == NULL) {
-	perror("tempfilename_generate: mktemp()");
+    if (mkstemp(tmp) == -1) {
+	perror("tempfilename_generate: mkstemp()");
 	exit(1);
     }
 

And for Tripwire-2.3.1 the patch is:

--- src/core/unix/unixfsservices.cpp.orig	Sat Feb 24 11:02:12
2001
+++ src/core/unix/unixfsservices.cpp	Tue Jul 10 21:40:37 2001
@@ -243,6 +243,7 @@
 {
     char* pchTempFileName;
     char szTemplate[MAXPATHLEN];
+    int fd;
 
 #ifdef _UNICODE
     // convert template from wide character to multi-byte string
@@ -253,13 +254,14 @@
     strcpy( szTemplate, strName.c_str() );
 #endif
 
-    // create temp filename
-    pchTempFileName = mktemp( szTemplate );
+    // create temp filename and check to see if mkstemp failed
+   if ((fd = mkstemp( szTemplate )) == -1) {
+     throw eFSServicesGeneric( strName );
+   } else {
+     close(fd);
+   }
+   pchTempFileName = szTemplate;
 
-    //check to see if mktemp failed
-    if ( pchTempFileName == NULL || strlen(pchTempFileName) == 0) {
-      throw eFSServicesGeneric( strName );
-    }
 
     // change name so that it has the XXXXXX part filled in
 #ifdef _UNICODE


We haven't had a chance to install the commercial version yet,
however  if the commercial version is vulnerable (I've notified
TripwireSecurity  of the possibility and I'm betting dollars to
donuts that is might be)  a possible workaround would be to create a
shared library with a  function named mktemp which would call
mkstemp() as in the patches  above, then execute tripwire using
LD_PRELOAD to load the mktemp  wrapper.


Regards,                         Phone:  (250)387-8437
Cy Schubert                        Fax:  (250)387-5766
Team Leader, Sun/Alpha Team   Internet:  Cy.Schubert@osg.gov.bc.ca
Open Systems Group, ITSD, ISTA
Province of BC
(6750452) /Cy Schubert - ITSD Open Systems Group <Cy.Schubert@uumail.gov.bc.ca>/(Ombruten)
Kommentar i text 6750420 av Jarno Huuskonen <Jarno.Huuskonen@uku.fi>
6750456 2001-07-12 02:51 +0300  /29 rader/ Lucian Hudin <luci@warp.transart.ro>
Sänt av: joel@lysator.liu.se
Importerad: 2001-07-16  08:12  av Brevbäraren
Extern mottagare: Alexandr Dubovikov <baron@uic-in.net>
Extern kopiemottagare: Przemyslaw Frasunek <venglin@freebsd.lublin.pl>
Extern kopiemottagare: Georgi Guninski <guninski@guninski.com>
Extern kopiemottagare: bugtraq@securityfocus.com
Mottagare: Bugtraq (import) <17982>
Kommentar till text 6734014 av Alexandr Dubovikov <baron@uic-in.net>
Ärende: Re: Re[2]: FreeBSD 4.3 local root, yet Linux and *BSD much better
------------------------------------------------------------
 than Windows
From: Lucian Hudin <luci@warp.transart.ro>
To: Alexandr Dubovikov <baron@uic-in.net>
Cc: Przemyslaw Frasunek <venglin@freebsd.lublin.pl>,
 Georgi Guninski <guninski@guninski.com>, <bugtraq@securityfocus.com>
Message-ID:
<Pine.LNX.4.30.0107120247420.32107-100000@warp.transart.ro>


> >> FreeBSD 4.3 local root, yet Linux and *BSD much better than Windows
>
> PF> This problem was already reported to FreeBSD Security Officer about two
> PF> months ago, but it was totally ignored.
>
> This problem has fixed and the exploit didn't work for last
> 4.3-RELEASE FreeBSD.
>
nope, it works like a charm on the 4.3-REL, maybe you forgot to
"cp /bin/sh /tmp/sh" first, but it works.
ftp.freebsd.org has:
$ less FreeBSD-SA-01:42.signal.asc
This is a place holder until the real advisory is issued.  One should not
reply on anything contained here.

FreeBSD Security Team

Regards,
LucySoft
(6750456) /Lucian Hudin <luci@warp.transart.ro>/(Ombruten)