Re: bzip2 TOCTOU file-permissions vulnerability

[Steve Grubb <linux_4ever@xxxxxxxxx>]

In-Reply-To: <20050402183613.GA26660@xxxxxxxxxxxxxxxxx>

Since no one's posted a fix...here's a stab at it.

-Steve Grubb

===============

diff -urb bzip2-1.0.2.orig/bzip2.c bzip2-1.0.2/bzip2.c
--- bzip2-1.0.2.orig/bzip2.c    2005-04-14 08:45:35.013302280 -0400
+++ bzip2-1.0.2/bzip2.c 2005-04-14 09:16:33.733733824 -0400
@@ -312,6 +312,7 @@

 static void    copyFileName ( Char*, Char* );
 static void*   myMalloc     ( Int32 );
+static int     applySavedFileAttrToOutputFile ( int fd );



@@ -457,6 +458,10 @@
    ret = fflush ( zStream );
    if (ret == EOF) goto errhandler_io;
    if (zStream != stdout) {
+      int fd = fileno ( zStream );
+      if (fd < 0) goto errhandler_io;
+      ret = applySavedFileAttrToOutputFile ( fd );
+      if (ret != 0) goto errhandler_io;
       ret = fclose ( zStream );
       outputHandleJustInCase = NULL;
       if (ret == EOF) goto errhandler_io;
@@ -567,6 +572,12 @@

    closeok:
    if (ferror(zStream)) goto errhandler_io;
+   if ( stream != stdout) {
+      int fd = fileno ( stream );
+      if (fd < 0) goto errhandler_io;
+      ret = applySavedFileAttrToOutputFile ( fd );
+      if (ret != 0) goto errhandler_io;
+   }
    ret = fclose ( zStream );
    if (ret == EOF) goto errhandler_io;

@@ -1125,7 +1136,7 @@


 static
-void applySavedMetaInfoToOutputFile ( Char *dstName )
+void applySavedTimeInfoToOutputFile ( Char *dstName )
 {
 #  if BZ_UNIX
    IntNative      retVal;
@@ -1134,16 +1145,26 @@
@@ -1134,16 +1145,26 @@
    uTimBuf.actime = fileMetaInfo.st_atime;
    uTimBuf.modtime = fileMetaInfo.st_mtime;

-   retVal = chmod ( dstName, fileMetaInfo.st_mode );
-   ERROR_IF_NOT_ZERO ( retVal );
-
    retVal = utime ( dstName, &uTimBuf );
    ERROR_IF_NOT_ZERO ( retVal );
+#  endif
+}
+
+static
+int applySavedFileAttrToOutputFile ( int fd )
+{
+#  if BZ_UNIX
+   IntNative      retVal;
+
+   retVal = fchmod ( fd, fileMetaInfo.st_mode );
+   if (retVal != 0)
+       return retVal;

-   retVal = chown ( dstName, fileMetaInfo.st_uid, fileMetaInfo.st_gid );
+   (void) fchown ( fd, fileMetaInfo.st_uid, fileMetaInfo.st_gid );
    /* chown() will in many cases return with EPERM, which can
       be safely ignored.
    */
+   return 0;
 #  endif
 }

@@ -1366,7 +1387,7 @@

    /*--- If there was an I/O error, we won't get here. ---*/
    if ( srcMode == SM_F2F ) {
-      applySavedMetaInfoToOutputFile ( outName );
+      applySavedTimeInfoToOutputFile ( outName );
       deleteOutputOnInterrupt = False;
       if ( !keepInputFiles ) {
          IntNative retVal = remove ( inName );
@@ -1544,7 +1565,7 @@
    /*--- If there was an I/O error, we won't get here. ---*/
    if ( magicNumberOK ) {
       if ( srcMode == SM_F2F ) {
-         applySavedMetaInfoToOutputFile ( outName );
+         applySavedTimeInfoToOutputFile ( outName );
          deleteOutputOnInterrupt = False;
          if ( !keepInputFiles ) {
             IntNative retVal = remove ( inName );