bzip2 TOCTOU file-permissions vulnerability

[Imran Ghory]

================================
bzip2 TOCTOU file-permissions vulnerability
================================

Software: bzip2
Version: 1.0.2
Software URL: <http://sources.redhat.com/bzip2/>
Platform: Unix, Linux.
Vulnerability type: Time-of-Check-Time-Of-Use
Severity: Low, requires local attacker and badly set directory permissions.


Vulnerable software
====================

bzip2 1.0.2 and previous versions running on unix.

bzip2 1.0.2 compiled for Windows using lcc or MS Visual C++ is not effected.

Vulnerability
==============

If a malicious local user has write access to a directory in which a
target user is using bzip2 to extract or compress a file to then a
TOCTOU bug can be exploited to change the permission of any file
belonging to that user.

On decompressing bzip2 copies the permissions from the compressed
bzip2 file to the
uncompressed file. However there is a gap between the uncompressed
file being written (and it's file handler being close) and the
permissions of the file being changed.

During this gap a malicious user can remove the decompressed file and
replace it with a hard-link to another file belonging to the user.
bzip2 will then change the permissions on the hard-linked file to be
the same as that of the bzip2 file.

Fix
====

Ensure that any directory which is being used by bzip2 to
compress/decompress files is only writeable by the user or
alternatively set the sticky bit on the directory's permissions

[Steve Grubb]

In-Reply-To: <7389fc4b0503301338f74a428@mail.gmail.com>

>================================
>bzip2 TOCTOU file-permissions vulnerability
>================================
>
>Software: bzip2
>Version: 1.0.2
>Software URL: <http://sources.redhat.com/bzip2/>
>Platform: Unix, Linux.
>Vulnerability type: Time-of-Check-Time-Of-Use
>Severity: Low, requires local attacker and badly set
>directory permissions.


This is completely wrong. If you look at the code to bzip2.c, you'll see that it calls fopen_output_safely(). This function in turn does an

open(name, O_WRONLY|O_CREAT|O_EXCL, S_IWUSR|S_IRUSR);

This means the file is opened with perms 0600.

>Vulnerable software
>====================
>
>bzip2 1.0.2 and previous versions running on unix.
>
>bzip2 1.0.2 compiled for Windows using lcc or MS
>Visual C++ is not effected.

Wrong again. In fopen_output_safely, if BZ_UNIX is not set, the it resorts to fopen. Looking at the manual_3.html page, it says that on Win32 you set this define to 0. Therefore windows might be vulnerable.

>Vulnerability
>==============
>
>If a malicious local user has write access to a
>directory in which a target user is using bzip2 to
>extract or compress a file to then a TOCTOU bug can
>be exploited to change the permission of any file
>belonging to that user.

Wrong. They must be logged in as that user. Remember, perms on tmp file was 0600.

>Fix
>====
>
>Ensure that any directory which is being used by
>bzip2 to compress/decompress files is only writeable
>by the user or alternatively set the sticky bit on the
>directory's permissions

According to changelog, this was fixed in 1.0.2. So, 1.0.1 and earlier might be a problem. This report is wrong in almost all aspects.

-Steve Grubb

[Steve Grubb]

In-Reply-To: <20050402183613.GA26660@securityfocus.com>

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