====================================================================== Advisory : SMF upload XSS vulnerability Release Date : December 4th, 2006 Application : Simple Machines Forum Version : SMF 1.1 Final (and earlier versions) Platform : PHP Vendor URL : http://www.simplemachines.org Authors : Jessica Hope ( jessicasaulhope@xxxxxxxxxxxxxx ) : rotwang ( c.a.rotwang@xxxxxxxxxxxxxx ) ======================================================================= Overview Due to various failures in sanitising user input, it is possible to construct XSS attacks using files masquerading as images. ======================================================================= Discussion A often ignored XSS hazard sprouts from the Internet Explorer's habit to "guess" the type of displayed data, when mime-type and header do not match. This is especially dangerous in software allowing image uploads; the accepted counter-measure is to use getimagesize to guarantee that the correct mime-type is chosen. SMF's implementation of this check is faulty, as it can be overridden by simply setting the parameter "image". In that case, the file will be delivered with the type "image/gif", regardless of the file's content or name. Even an uploaded text file is able to carry an XSS vector. Neither the upload function, nor the delivery code actually act upon the file's content. The admin function "Check attachment's extension" has no impact on that behaviour. Vulnerable code in Display.php Line 1045 if (filesize($filename) != 0) { $size = @getimagesize($filename); if (!empty($size) && $size[2] > 0 && $size[2] < 4) header('Content-Type: image/' . ($size[2] != 1 ? ($size[2] != 2 ? 'png' : 'jpeg') : 'gif')); // Errr, it's an image.... what kind? A... gif? Yeah that's it, gif! Like JIF, the peanut butter. elseif (isset($_REQUEST['image'])) header('Content-Type: image/gif'); } ======================================================================= Solution It is possible to work around the issue like so: $size = @getimagesize($filename); if (!empty($size) && $size[2] > 0 && $size[2] < 4) { header('Content-Type: image/' . ($size[2] != 1 ? ($size[2] != 2 ? 'png' : 'jpeg') : 'gif')); } // Errr, it's not an image.... what kind? Ah, let's play it safe else { header('Content-Disposition: attachment; filename="' . $real_filename . '"'); header('Content-Type: application/octet-stream'); } Moreover, the upload function should check the actual filtype. Files with invalid extensions should not be accepted as uploads. The avatar function already implements such checks; they should be applied for the attachment function as well. ======================================================================= History: Having dealt with SMF in the past, I know that they do not take security seriously (have a look at my earlier IP spoofing SMF report). Thus until I believe that SMF have shaped up in terms of security, any issues that I come across I'll be posting a full disclosure immediately. Incidently, the IP spoofing still exsists in the latest SMF too. They never learn. 04th December 2006: Full disclosure ======================================================================= Credit This issue is to be credited to Jessica Hope ( jessicasaulhope@xxxxxxxxxxxxxx ), and rotwang ( c.a.rotwang@xxxxxxxxxxxxxx )
