QuickTime patched

Apple has just released a fix to a vulnerability I reported to them a while ago (go update to 7.5.5).

Quoting apple (http://support.apple.com/kb/HT1222)

"CVE-ID:  CVE-2008-3624
Available for:  Mac OS X v10.4.9 - v10.4.11,
Mac OS X v10.5 or later, Windows Vista, XP SP2 and SP3
Impact:  Viewing a maliciously crafted QTVR movie file may lead to an
unexpected application termination or arbitrary code execution
Description:  A heap buffer overflow exists in QuickTime's handling
of panorama atoms in QTVR (QuickTime Virtual Reality) movie files.
Viewing a maliciously crafted QTVR file may lead to an unexpected
application termination or arbitrary code execution. This update
addresses the issue through improved bounds checking of panorama
atoms.

In a nutshell, this vulnerability is caused by a Sign Extension bug which leads to a heap overflow.

I would like to spit out a few words regarding sign extension bugs because I feel they are somewhat overlooked

What are sign extension bugs?

Let's take a look at a vulnerable code snippet:

 

   1:  char target[0xFFFF];

   2:  char src[0xFFFF];

   3:  ....

   4:  short x = <user input>;

   5:  long y = x;

   6:  memset(target, 1, y);

At line [4], x is defined as short which is assigned by user input.
At line [5], x is extended into long, and copied to y.
At line [6]. y is used as the length parameter for the memset call.

So where is the overflow?

To begin with, for the sake of simplicity, short is 16bit and long is 32bit.

Since we are dealing with signed numbers, the extension from short to long must not affect the sign of the input.

If x is above or equals to 0x8000 (-32768), the most significant bits of y are set (e.g: 0x8000 becomes 0xFFFF8000).  Under x86, this is done by the  MOVSX instruction (As opposed to MOVZX which is used for extending unsigned numbers).

When there is a sign/unsign mismatch, a negative signed number is effectively considered as a very large unsigned number. This exactly what happens in the memset function as the length parameter is expected to be unsigned.

Exploitation:

Set  x to a value above or equal to 0x8000, This will make y a very large unsigned number, and cause memset to overflow the given target.

Best practice:

Don't mix up signed and unsigned numbers. Passing a signed number to a function which receives an unsigned one is prone to errors.

In order to fix the code snippet above one should define x and y as unsigned numbers instead of signed ones. This will make the compiler use the MOVZX instruction instead of MOVSX, making it impossible for a malicious user to assign a value larger than 0xFFFF to the long variable.

 

Acknowledgments:

I would like to thank Apple Product Security team for their quick responses and the efficient way in which they handled this security issue.