Re: Tripwire temporary files

2001-07-16 Thread Cy Schubert - ITSD Open Systems Group

In message [EMAIL PROTECTED], 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 SchubertFax:  (250)387-5766
Team Leader, Sun/Alpha Team   Internet:  [EMAIL PROTECTED]
Open Systems Group, ITSD, ISTA
Province of BC






Re: Tripwire temporary files

2001-07-15 Thread Jarno Huuskonen

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 XX 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 [EMAIL PROTECTED]



Re: Tripwire temporary files

2001-07-15 Thread Cy Schubert - ITSD Open Systems Group

In message [EMAIL PROTECTED], 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(twtempXX);  
 - 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 voiddatabase_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.cTue 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/twzXX);
 
-   if ((char *) mktemp(tmpfilename) == NULL) {
-   perror(siggen: mktemp());
+   if (mkstemp(tmpfilename) == -1) {
+   perror(siggen: mkstemp());
exit(1);
}
 
--- src/utils.c.origTue 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) 

Re: Tripwire temporary files

2001-07-10 Thread Charles Stevenson

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(twtempXX);  
- 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.

Best Regards,
Charles Stevenson



Re: Tripwire temporary files

2001-07-10 Thread Paul Starzetz

Jarno Huuskonen wrote:

  I found out about the problem when I noticed a temporary file
  /tmp/twtempa19212 left in /tmp. Out of curiosity I ran the tripwire
  binary with strace and noticed that temporary files in /tmp are opened
  without the O_EXCL flag.

Here a strace from tripwire 1.2 (Source RPM: tripwire-1.2-223.src.rpm):

open(/tmp/twznG1Eud, O_RDWR|O_CREAT|O_TRUNC, 0666) = 4
open(/tmp/twzd9tWqg, O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
open(/tmp/twzzykpkj, O_RDWR|O_CREAT, 0600) = 4

nowhere the current pid is used - instead a 6 byte template appears,
which is not really predictable (at least shouldn't be!).

Ihq.



Re: Tripwire temporary files

2001-07-10 Thread Jarno Huuskonen

On Tue, Jul 10, Paul Starzetz wrote:
 Jarno Huuskonen wrote:
 
   I found out about the problem when I noticed a temporary file
   /tmp/twtempa19212 left in /tmp. Out of curiosity I ran the tripwire
   binary with strace and noticed that temporary files in /tmp are opened
   without the O_EXCL flag.
 
 Here a strace from tripwire 1.2 (Source RPM: tripwire-1.2-223.src.rpm):
 
 open(/tmp/twznG1Eud, O_RDWR|O_CREAT|O_TRUNC, 0666) = 4
 open(/tmp/twzd9tWqg, O_WRONLY|O_CREAT|O_TRUNC, 0666) = 3
 open(/tmp/twzzykpkj, O_RDWR|O_CREAT, 0600) = 4
 
 nowhere the current pid is used - instead a 6 byte template appears,
 which is not really predictable (at least shouldn't be!).

So that version of tripwire is not compiled with glibc that uses a
letter + pid as the unique/random part. I only mentioned that the binary
version of tripwire (2.2.1) avalaible from www.tripwire.com does that.
But as you can see it doesn't use O_EXCL so if the 'random' file happens
to be a symlink tripwire will overwrite files.

-Jarno



Tripwire temporary files

2001-07-09 Thread Jarno Huuskonen

  
  Insecure temporary files in Tripwire
 [EMAIL PROTECTED]
  $Date: 2001/07/09 05:02:02 $
  

Author:
 Jarno Huuskonen [EMAIL PROTECTED]

Discovered:
 Tue 16 Jan 2001

Vendor status:
 Vendor contacted on Tue, 16 Jan 2001.
 Fixed version available from: http://sourceforge.net/projects/tripwire/

Platforms:
 Tripwire 2.2.1, tripwire 2.3.0 (and tripwire-ASR-1.3.1). I tested the
 vulnerability only on Linux, so I can't confirm if other platforms or
 versions are affected or not (but I believe that versions2.4 are).

Severity:
 Possible local file overwrite (symlink attack). (For more information
 about race conditions see[1][2][3]).

Abstract:
 Tripwire opens/creates temporary files insecurely. Insecure temporary
 files are created at least when scanning the filesystem and updating
 tripwire database.

Details:
 Tripwire opens/creates temporary files in /tmp without O_EXCL flag.
 These temporary filenames are created with mktemp systemcall. This
 makes it possible for a local user to overwrite files with root (user
 running tripwire) permissions.

 I found out about the problem when I noticed a temporary file
 /tmp/twtempa19212 left in /tmp. Out of curiosity I ran the tripwire
 binary with strace and noticed that temporary files in /tmp are opened
 without the O_EXCL flag.

 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).

 Insecure files are created at least when running:
  tripwire --check
  tripwire --update -r reportdir/reportfile
  tripwire --check --email-report

 Tripwire-2.2.1 for Linux (the binary version available from
 http://www.tripwire.com) seems to be the easiest to exploit because
 it is statically linked with mktemp that uses the PID of tripwire for
 creating the 'unique' filename. In all of my tests with this version
 the temporary filename was /tmp/twtempaPID.

Solution:
 Install fixed tripwire and use the new TEMPDIRECTORY configuration
 option so tripwire can use only root writable temporary directory
 (e.g. /root/tmp).

 Note: tripwire-2.3.1-2 (from sourceforge) still has one unsafe
 temporary file open (see tripwire-2.3.1-2/src/core/archive.cpp
 cLockedTemporaryFileArchive::OpenReadWrite).

Possible workaround:
 It appears possible to patch binary tripwire (2.2.1) to create
 temporary files in a directory other than /tmp. To do this edit the
 tripwire binaries: siggen, tripwire, twadmin and twprint, and replace
 the twtempXX strings with e.g. tw/tmpXX. Now tripwire creates
 temporary files in /tmp/tw directory (The /tmp/tw directory should be
 owned by root and writable only by root). Note: If you're going to edit
 the binaries do it at your own risk AND read the tripwire license
 before doing so! (and only if you can't get an updated tripwire).

Credits:
 I would like to thank Ron Forrester from Tripwire for handling my bug
 report.

References:
1.
 David A. Wheeler: Secure Programming for Linux and Unix HOWTO.
 http://www.dwheeler.com/secure-programs/Secure-Programs-HOWTO/avoid-race.html

2.
 Kris Kennaway's post to Bugtraq about temporary files.
 http://lwn.net/2000/1221/a/sec-tmp.php3

3.
 Creating Secure Software: 
 http://www.eforceglobal.com/pdf/whitepapers/SecureSoftware-01-10-01-FINAL.pdf

-- 
Jarno Huuskonen [EMAIL PROTECTED]