<!-- SBZ_reproduce  -->
http://scary.beasts.org/misc/bad.bmp

Critical priority, could be embedded in webpages (konqueror crashes), not to 
mention HTML mail. 
 
Adrian, this might also affect older SL / SLES releases. 

no patch yet around ? 
 

Created attachment 22441 [details]
fix

The image specifies a size of 1x1 but then got a much larger image data. This
is read as chunk into the preallocated buffer (for a 1x1 image). So I would
guess, there is at least the possibility to exploit this.

Is qt-bugs@trolltech.com notified? Can someone from the security team please 
coordinate with them or at least bounce the report+patch? 

Created attachment 22446 [details]
other fix

The memset at line 4849:
       memset( p, d->getch(), b ); // repeat pixel
can go wrong as well and remaining_size should be updated afterwards there as
well. Instead of introducing a new state variable in the form of relaining_size
I suggest to use "(bpl - x)" instead, see attached patch.

comment #5, i'll ask the reporter about a probably established contact to the 
trolltech folks. 

is 'b' signed? 

yes, int b = getch(); 

I don't know the whole code but it's better to check if b is negativ. 
 
if ( b <= 0 || b > (bpl - x) ) 

For the sake of completeness: 
Date: Wed, 28 Jul 2004 16:24:01 +0100 (BST) 
From: chris@scary.beasts.org 
To: Thomas Biege <thomas@suse.de> 
Cc: Mark J Cox <mjc@redhat.com>, vendor-sec@lst.de 
Subject: Re: [vendor-sec] QT BMP decoder heap overflow vulnerability 
 
Hi, 
 
On Wed, 28 Jul 2004, Thomas Biege wrote: 
 
> On Wed, 28 Jul 2004, Mark J Cox wrote: 
> 
> > > qt BMP parser heap overflow error 
> > > Demo BMP: http://scary.beasts.org/misc/bad.bmp (flaw 1a). 
> 
> Is someone in contact with the Trolltech people (qt-bugs@trolltech.com)? 
 
Yes, I've taken care of this. Sorry if I didn't make this clear with my 
initial post. 
 
I have just heard back from QT's support manager, Volker Hilsheimer. He 
has provided a patch, which looks good on initial read. I've included it 
below. 
 
I see no reason to deviate from the 18th Aug disclosure date. Mark doesn't 
have a specific time of day down, so let's use the favoured time of 
1400UTC. 
 
Cheers 
Chris 
 
--- src/kernel/qimage.cpp      2004-07-28 13:32:21 -0000 
+++ src/kernel/qimage.cpp      2004-07-28 13:32:21 -0000 
 
@@ -4827,6 +4827,7 @@ 
       if ( comp == BMP_RLE8 ) {       // run length compression 
           int x=0, y=0, b; 
           register uchar *p = line[h-1]; 
+          const uchar *endp = line[h-1]+w; 
           while ( y < h ) { 
       if ( (b=d->getch()) == EOF ) 
           break; 
@@ -4844,9 +4845,20 @@ 
       case 2: // delta (jump) 
           x += d->getch(); 
           y += d->getch(); 
+ 
+          // Protection 
+          if ( (uint)x >= (uint)w ) 
+      x = w-1; 
+          if ( (uint)y >= (uint)h ) 
+      y = h-1; 
+ 
           p = line[h-y-1] + x; 
           break; 
       default:// absolute mode 
+          // Protection 
+          if ( p + b > endp ) 
+      b = endp-p; 
+ 
           if ( d->readBlock( (char *)p, b ) != b ) 
       return FALSE; 
           if ( (b & 1) == 1 ) 
@@ -4855,6 +4867,10 @@ 
           p += b; 
           } 
       } else {// encoded mode 
+          // Protection 
+          if ( p + b > endp ) 
+      b = endp-p; 
+ 
           memset( p, d->getch(), b ); // repeat pixel 
           x += b; 
           p += b; 
 

I forgot the full post: 
 
Date: Wed, 28 Jul 2004 00:59:37 +0100 (BST) 
From: chris@scary.beasts.org 
To: vendor-sec@lst.de 
Subject: [vendor-sec] QT BMP decoder heap overflow vulnerability 
 
Hi, 
 
I've appended below an advisory for a heap overflow in QT's BMP decoder. 
Of course, KDE (incl. konqueror, kmail etc). uses QT to handle BMP files. 
 
I have reported the bug to the QT people, and have suggested a disclosure 
date of Aug 18th. I'll report back if/when they send me details of the fix 
patch. 
 
I have also thought of another attack vector for image decoding bugs - the 
face browsers where GDM / KDM allow users to supply their own login 
images. Do any distributions enable this functionality by default? Ages 
ago their was talk of applying priv-sep to GDM / KDM so that things such 
as X protocol handling and image decoding would run under a non-privileged 
user. Did either GDM / KDM get around to implementing this? 
 
Cheers 
Chris 
 
CESA-2004-004 - rev 1 
 
qt BMP parser heap overflow error 
================================= 
 
Programs affected: qt, and any programs which use qt to decode BMP files. For 
example, KDE (including konqueror). 
Severity: Possible compromise of account used to browse malicious BMP files. 
 
This advisory notes a code flaws discovered by inspection of the qt code. 
The specific version of qt discussed is v3.3.2. 
 
Flaw 1. Heap-based overflow in read_dib (qimage.cpp). 
 
The handling of 8-bit RLE encoded BMP files is faulty. Interestingly, the 
4-bit 
RLE encoding handling seems to have the required safety checks. 
a) User supplied length used to read into heap buffer without adequate bounds 
checking: 
     default:    // absolute mode 
          if ( d->readBlock( (char *)p, b ) != b ) 
b) User supplied length used to memset() a piece of heap buffer without 
adequate bounds checking: 
    } else {      // encoded mode 
        memset( p, d->getch(), b ); // repeat pixel 
c) User supplied delta pixel co-ordinates used without range checking: 
      case 2:     // delta (jump) 
          x += d->getch(); 
          y += d->getch(); 
          p = line[h-y-1] + x; 
 
Demo BMP: http://scary.beasts.org/misc/bad.bmp (flaw 1a). 
 
 
CESA-2004-004 - rev 1 
Chris Evans 
chris@scary.beasts.org 
 
[Advertisement: I am interested in moving into a security related field 
 full-time. E-mail me to discuss.] 
 
_______________________________________________ 
Vendor Security mailing list 
Vendor Security@lst.de 
https://www.lst.de/cgi-bin/mailman/listinfo/vendor-sec 
 

The XPM reader can also crash.                                                   
                                                                                 
Just change the 3rd argument of the image definition to be negative.             
                                                                                 
Testcase:                                                                        
        /* XPM */                                                                
        static char * foo_xpm[] = {                                              
        "1 1 -1 1",                                                              
        "       c #FFFFFF",                                                      
        " "};                                                                    
                                                                                 
read_xpm_image_or_array does:                                                    
    if ( sscanf( buf, "%d %d %d %d", &w, &h, &ncols, &cpp ) < 4 )                
        return;                                 // < 4 numbers parsed            
                                                                                 
    if ( cpp > 15 )                                                              
        return;                                                                  
                                                                                 
    if ( ncols > 256 ) {                                                         
        image.create( w, h, 32 );                                                
    } else {                                                                     
        image.create( w, h, 8, ncols );                                          
    }                                                                            
                                                                                 
ncols -1 -> image.create fails, gets NULL pointers to write into later.          
                                                                                 
The return value of image.create should be checked here, also cpp < 0 and        
ncols < 0.                                                                       
                                                                                 
Not exploitable, just an easy DoS.                                               

Re #9: getch() returns values in the range of 0-255 but can also return -1 on 
EOF. 

and the EOF case is handled earlier in the code as coolo told me.  

> The XPM reader can also crash. 
> 
> Just change the 3rd argument of the image definition to be negative. 
 
that XPM crash is CAN-2004-0692 

CAN-2004-0693 for the qt GIF crasher 
 

testcase: 
All I've been able to find is a crash, looks like a NULL pointer crash if 
a GIF frame has no size but still has image data. Demo is at: 
http://scary.beasts.org/misc/crash.gif 

was there any answer from the Trolls yet ? Do/Will they provide official 
patches ? 

comment #11 is all i got. 

AFAICR Marcus contacted the trolltech folks for the XPM issue he found. 

i did not have any feedback yet. 

Created attachment 22598 [details]
patch from official qt 3.3.3 commercial edition for qimage.cpp. can you all review it for the BMP and XPM issues please ?

from the 3.3.3 changelog: 
 
- QImage 
        Included fix for buffer overflow in libPNG. 
        Fixed bug that made copy constructor not copy the entire image. 
        Allow XPM images with colors that have more than one word in the 
        name. 
        Fixed crash when trying to load a corrupt/invalid BMP image. 
        Fixed crash when trying to load a corrupt/invalid GIF image. 
        Fixed crash when trying to load a JPEG image that is too big. 
        Fixed bug that caused dotsPerMeter() to be ignored when saving 
        JPEG images. 
 

i dont think it fixes my XPM crash. 
 
can you also quote the qasyncimage* part for the GIF fixes? 

Created attachment 22599 [details]
qasyncimageio.cpp.diff

for xpm: 
 
Ok, this issue has been fixed now with the following patch:                      
                                                                                 
==== //depot/qt/3/src/kernel/qimage.cpp#26 - c:                                  
\work\depot\qt\3\src\kernel\qimage.cpp ====                                      
@@ -5723,6 +5723,9 @@                                                            
        image.create( w, h, 8, ncols );                                          
     }                                                                           
                                                                                 
+    if (image.isNull())                                                         
+        return;                                                                 
+                                                                                
     QMap<QString, int> colorMap;                                                
     int currentColor;                                                           
                                                                                 
                                                                                 
Thanks for the report and have a nice day!                                       
                                                                                 
Andy                                                                             

okay, so we are secure again with these both patches and I shall start 
backporting ? 
 
 

the gif part does not look complete yet. I asked on vendor-sec for 
full patches. 

The stuff seems public now.. it even fixes a problem in the JPEG code! 
 
Date: Wed, 11 Aug 2004 22:01:13 +0100 
From: Chris Evans <chris@scary.beasts.org> 
To: vendor-sec@lst.de 
Subject: [vendor-sec] qt-3.3.3 
 
Hi- 
 
The latest qt-3.3.3 release, unfortunately, refers to 
all the image loader crashes. Including the possibly 
serious BMP one. A JPG crash on "oversized" image is 
also mentioned. I don't have time to investigate but 
hopefully its a case of a missing check for the image 
::create() call leaving a null image when the x,y 
dimensions are too big. (Can someone check? I'm away 
from home at the moment and have limited time and 
access). 
 
Does this effectively lift the embargo for the QT 
issues, i.e. update when ready? What policy applies to 
premature public disclosure of embargoed issues? 
 
Cheers 
Chris 
 

Created attachment 22674 [details]
parts between 3.3.2 and 3.3.3 that seem to be relevant.

Created attachment 22675 [details]
XPM fix 2

adrian, we need the last 2 patches. 

working on it 

everything is prepared and looks good now .... but not the GIF problem. it is 
still crashing with the example file from Marcus. 

Created attachment 22680 [details]
gif fix for crashing gif... from Adrian

packages with these patches has been submitted. 
 
esp. the jpeg support on SLES8 should be tested a bit more extensive, since 
this code needed a bit more changes. 
 
patchinfos not yet created. 

Created attachment 22691 [details]
patchinfo-box.qt

Created attachment 22692 [details]
patchinfo.qt

packages are checked in, patchinfos are submitted. 

testing (mmichna) found problems yesterday. 
 
+ konqueror no longer crashes. 
- However, kio_thumbnail still crashes in the example directory, which 
  might be a second stage problem. 
 
adrian is currently debugging this problem. 

the SLEC issue is a bug in kio_thumbnail, which got fixed since 3.2.1. 
 
I guess we can ignore this issue here, since it is not really easy to trigger 
this remotely. 
 
-r 1.33 kdebase/kioslave/thumbnail/thumbnail.cpp 

at least it shall not block the QT update. 
 
is there status for the other platforms? 

packages approved, advisory released. 
 
 

<!-- SBZ_reopen -->Reopened by thomas@suse.de at Fri Aug 20 13:45:35 2004

reopenend due to problems on 8.1/sles8. 
 
Adrian already sumbitted new packages. 

<!-- SBZ_reopen -->Reopened by lnussel@suse.de at Fri Aug 20 13:46:14 2004, took initial reporter thomas@suse.de to cc

packages aproved