Re: Tripwire temporary files

From: Cy Schubert - ITSD Open Systems Group (Cy.Schubertat_private)
Date: Thu Jul 12 2001 - 18:56:42 PDT

  • Next message: Jarno Huuskonen: "Re: Tripwire temporary files"

    In message <3B4A936D.FF2DA075at_private>, 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.Schubertat_private
    Open Systems Group, ITSD, ISTA
    Province of BC
    



    This archive was generated by hypermail 2b30 : Thu Jul 12 2001 - 21:52:49 PDT