Wednesday, December 17, 2008

NULL pointer exploitation

Today Sun finally released a fix for a kernel bug I reported to them back in 2007 (TKADV2008-015). The vulnerability is a NULL pointer dereference that can be reliably exploited under x86 platforms (32 and 64bit).

This movie demonstrates the exploitability of the vulnerability. The exploit first escapes from a restricted non-global Solaris zone back into the global zone, drops all restrictions and then gains root privileges.

# mdb -k unix.4 vmcore.4
Loading modules: [ unix krtld genunix specfs dtrace cpu.AuthenticAMD.15 uppc
pcplusmp ufs ip sctp usba fcp fctl nca lofs audiosup zfs random sppp crypto md 
cpc fcip logindmux ptm nfs ]

> $c
0x44434241(c, dad0ebc0) 
ip_sioctl_tunparam+0xfc(0, d46c0200, d6d352b0, d6849380, fecc6e80, d6d352b0)
ip_process_ioctl+0x2a9(0, d6d352b0, d6849380, fecc6e80)
ip_wput_nondata+0x248(0, d6d352b0, d6849380, 0)
ip_output+0x376()
ip_wput+0x14(d6d352b0, d6849380)
putnext+0x1b7(d6d352b0, d6849380)
ar_wput+0x131(d6d31430, d6849380)
putnext+0x1b7(d6d31430, d6849380)
strdoioctl+0x4f1(d6218508, d55d6d10, 0, 100003, 1, d60225b0)
strioctl+0x3b1(d6d64a80, 40586993, 8061020, 100003, 1, d60225b0)
spec_ioctl+0x48(d6d64a80, 40586993, 8061020, 100003, d60225b0, d55d6f78)
fop_ioctl+0x24(d6d64a80, 40586993, 8061020, 100003, d60225b0, d55d6f78)
ioctl+0x199()
sys_sysenter+0x100()

> $r
%cs = 0x0158            %eax = 0x44434241   
%ds = 0xd4d80160        %ebx = 0x000007d0
%ss = 0x000c            %ecx = 0x000007d0
%es = 0x0160            %edx = 0xd561c000
%fs = 0x0000            %esi = 0x00000000
%gs = 0x01b0            %edi = 0x00000000

%eip = 0x44434241  
%ebp = 0xd55d6a8c
%esp = 0xd55d6a48

Sunday, December 14, 2008

Stack Buffer Overflow in MPlayer

I just released an advisory (TKADV2008-014) describing the details of a stack buffer overflow vulnerability I found in the current versions of MPlayer. See the diff of the SVN trunk.

Thursday, December 04, 2008

Tricky Integer Arithmetic

Even that it is known for quite a while that arithmetic errors can lead to integer overflow vulnerabilities (in C) these security issues still tend to confuse a lot of C programmers. A good example is the latest vulnerability I found in the VLC media player: an integer overflow vulnerability that leads to an exploitable heap overflow (see TKADV2008-013 for a detailed description).

The vulnerability itself is quite straight forward:

[...]
891 static void ReadRealIndex( demux_t *p_demux )
892 {
...
900      uint32_t      i_index_count;
...
920 [1]  i_index_count = GetDWBE( &buffer[10] );
...
931 [2]  p_sys->p_index = 
932            (rm_index_t *)malloc( sizeof( rm_index_t ) * (i_index_count+1) );
...
938 [3]  for( i=0; i < i_index_count; i++ )
939      {
940         if( stream_Read( p_demux->s, buffer, 14 ) < 14 )
941             return ;
942
943 [7]     if( GetWBE( &buffer[0] ) != 0 )
944         {
945            msg_Dbg( p_demux, "Real Index: invaild version of index entry %d ",
946                               GetWBE( &buffer[0] ) );
947            return;
948         }
949
950 [4]     p_sys->p_index[i].time_offset = GetDWBE( &buffer[2] );
951 [5]     p_sys->p_index[i].file_offset = GetDWBE( &buffer[6] );
952 [6]     p_sys->p_index[i].frame_index = GetDWBE( &buffer[10] );
[...]

[1] User supplied data from the RealMedia file gets copied into "i_index_count".

[2] The value of "i_index_count" is used to calculate the size of a heap buffer. If the value of "i_index_count" is big enough (e.g. 0x15555555) an integer overflow occurs while calculating the size of the heap buffer. As a consequence it is possible to allocate a small heap buffer by supplying a big value for "i_index_count".

[3] The value of "i_index_count" is used as a counter in this for() loop.

[4] User controlled data from the RealMedia file gets copied into the previously allocated heap buffer (see [2]). As "i" is used as an array index and the for() loop is executed until "i < i_index_count" it is possible to overflow the heap buffer with user controlled data from the RealMedia file. [5] See [4] [6] See [4] As there is also an exit condition that can be triggered to stop the overflow (see [7]) at any given point this leads to a fully controllable heap overflow that can be exploited by a (remote) attacker to execute arbitrary code in the context of VLC. The VideoLAN team "fixed" the vulnerability with the following patch (original link):
diff --git a/modules/demux/real.c b/modules/demux/real.c
index 7574739..ddfb64d 100644
--- a/modules/demux/real.c
+++ b/modules/demux/real.c
@@ -932,16 +932,13 @@ static void ReadRealIndex( demux_t *p_demux )
msg_Dbg( p_demux, "Real Index: Does next index exist? %d ",
GetDWBE( &buffer[16] )  );

-    p_sys->p_index =
-      (rm_index_t *)malloc( sizeof( rm_index_t ) * (i_index_count+1) );
+    p_sys->p_index = calloc( i_index_count + 1, sizeof( rm_index_t ) );
if( p_sys->p_index == NULL )
{
msg_Err( p_demux, "Memory allocation error" ); 
return;
}

-    memset( p_sys->p_index, 0, sizeof(rm_index_t) * (i_index_count+1) );
-
for( i=0; i < i_index_count; i++ )
{
if( stream_Read( p_demux->s, buffer, 14 ) < 14 )
The main difference is that the VLC maintainers exchanged malloc() with calloc(). The idea itself is not bad: calloc() was "hardened" on different platforms so that integer overflows due to multiplication within calloc() should be catched. But in this case the integer overflow still happens as the first argument to calloc() is calculated with the addition "i_index_count + 1". A user supplied "i_index_count" of UINT_MAX (0xffffffff) will cause an integer overflow within the first parameter of calloc() and therefore only allocate a 0 byte buffer. Please notice that calloc(0, sizeof(rm_index_t)) will not return a NULL pointer but a pointer into the legal heap on at least platforms like Windows and Linux. Later on the "i_index_count" value is used as a counter in the for() loop. This still leads to an exploitable heap overflow. Well, I reported this issue back to the VLC maintainers resulting in the following "second patch" (original link):
diff --git a/modules/demux/real.c b/modules/demux/real.c
index ddfb64d..cfadef2 100644
--- a/modules/demux/real.c
+++ b/modules/demux/real.c
@@ -925,14 +925,14 @@ static void ReadRealIndex( demux_t *p_demux )

msg_Dbg( p_demux, "Real Index : num : %d ", i_index_count );

-    if( i_index_count == 0 )
+    if( i_index_count > ( 0xffffffff / sizeof( rm_index_t ) ) )
return;

if( GetDWBE( &buffer[16] ) > 0 )
msg_Dbg( p_demux, "Real Index: Does next index exist? %d ",
GetDWBE( &buffer[16] )  );

-    p_sys->p_index = calloc( i_index_count + 1, sizeof( rm_index_t ) );
+    p_sys->p_index = malloc( ( i_index_count + 1 ) * sizeof( rm_index_t ) );
if( p_sys->p_index == NULL )
{
msg_Err( p_demux, "Memory allocation error" ); 
@@ -954,12 +954,13 @@ static void ReadRealIndex( demux_t *p_demux )
p_sys->p_index[i].time_offset = GetDWBE( &buffer[2] );
p_sys->p_index[i].file_offset = GetDWBE( &buffer[6] );
p_sys->p_index[i].frame_index = GetDWBE( &buffer[10] );
-        msg_Dbg( p_demux, "Real Index: time %d file %d frame %d ",
-                        p_sys->p_index[i].time_offset,
-                        p_sys->p_index[i].file_offset,
-                        p_sys->p_index[i].frame_index );
-
+        msg_Dbg( p_demux,
+                 "Real Index: time %"PRIu32" file %"PRIu32" frame %"PRIu32,
+                 p_sys->p_index[i].time_offset,
+                 p_sys->p_index[i].file_offset,
+                 p_sys->p_index[i].frame_index );
}
+    memset( p_sys->p_index + i_index_count, 0, sizeof( rm_index_t ) );
}
In this patch the malloc() function is used again instead of calloc(). But what is more interesting is the security check that was introduced with this second patch: if( i_index_count > ( 0xffffffff / sizeof( rm_index_t ) ) ). It is now checked if ( 0xffffffff / sizeof( rm_index_t ) ) is smaller than the user supplied value of i_index_count. If so, the heap buffer will not be allocated and the vulnerability can't be reached anymore. The result of the division ( 0xffffffff / sizeof( rm_index_t ) ) is 0x15555555. Now what happens if i_index_count has a value of 0x15555555? Well, the security check can be successfully bypassed and the integer overflow happens again in the calculation of the size argument of malloc(): p_sys->p_index = malloc( ( i_index_count + 1 ) * sizeof( rm_index_t ). I also reported that back to the VLC maintainers. As always, the VLC maintainers reacted promptly and provided another patch to fix the issue. It looks like the following (original link):
diff --git a/modules/demux/real.c b/modules/demux/real.c
index ef3a816..fa5f1ee 100644
--- a/modules/demux/real.c
+++ b/modules/demux/real.c
@@ -921,7 +921,7 @@ static void ReadRealIndex( demux_t *p_demux )

msg_Dbg( p_demux, "Real Index : num : %d ", i_index_count );

-    if( i_index_count > ( 0xffffffff / sizeof( rm_index_t ) ) )
+    if( i_index_count >= ( 0xffffffff / sizeof( rm_index_t ) ) )
return;

if( GetDWBE( &buffer[16] ) > 0 )

Is the vulnerability now *really* fixed with this patch? As I said, integer arithmetic can be quite tricky :>