Bug#509596: roundcube: CVE-2008-5620 massive memory consumption via crafted image

[Nico Golde]

Package: roundcube
Severity: grave
Tags: security patch

Hi,
the following CVE (Common Vulnerabilities & Exposures) id was
published for roundcube.

CVE-2008-5620[0]:
| RoundCube Webmail (roundcubemail) before 0.2-beta allows remote
| attackers to cause a denial of service (memory consumption) via
| crafted size parameters that are used to create a large quota image.

Attached is a patch I extracted from the bundled upstream 
patch on http://sourceforge.net/forum/forum.php?forum_id=898542

If you fix the vulnerability please also make sure to include the
CVE id in your changelog entry.

For further information see:

[0] http://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2008-5620
    http://security-tracker.debian.net/tracker/CVE-2008-5620

-- 
Nico Golde - http://www.ngolde.de - n...@jabber.ccc.de - GPG: 0x73647CFF
For security reasons, all text in this mail is double-rot13 encrypted.

--- quotaimg.php.old	2008-12-23 18:06:58.000000000 +0100
+++ quotaimg.php	2008-12-16 18:21:30.000000000 +0100
@@ -14,14 +14,14 @@
  | Author: Brett Patterson <bre...@umbc.edu>                             |
  +-----------------------------------------------------------------------+
 
- $Id: $
+ $Id$
 
 */
 
-$used   = ((isset($_GET['u']) && !empty($_GET['u'])) || $_GET['u']=='0')?(int)$_GET['u']:'??';
-$quota  = ((isset($_GET['q']) && !empty($_GET['q'])) || $_GET['q']=='0')?(int)$_GET['q']:'??';
-$width  = empty($_GET['w']) ? 100 : (int)$_GET['w'];
-$height = empty($_GET['h']) ? 14 : (int)$_GET['h'];
+$used   = isset($_GET['u']) ? intval($_GET['u']) : '??';
+$quota  = isset($_GET['q']) ? intval($_GET['q']) : '??';
+$width  = empty($_GET['w']) ? 100 : min(300, intval($_GET['w']));
+$height = empty($_GET['h']) ? 14  : min(50,  intval($_GET['h']));
 
 /**
  * Quota display
@@ -96,12 +96,11 @@
     // @todo: Set to "??" instead?
 	if (ereg("^[^0-9?]*$", $used) || ereg("^[^0-9?]*$", $total)) {
 		return false; 
-    }
+	}
 
-	if (strpos($used, '?') !== false || strpos($total, '?') !== false
-        && $used != 0) {
+	if (strpos($used, '?') !== false || strpos($total, '?') !== false && $used != 0) {
 		$unknown = true; 
-    }
+	}
 
 	$im = imagecreate($width, $height);
 
@@ -153,31 +152,37 @@
 			list($r, $g, $b) = explode(',', $color['fill']['mid']);
 			$fill = imagecolorallocate($im, $r, $g, $b);
 		} else {
-		    // if($quota >= $limit['low'])
+			// if($quota >= $limit['low'])
 			list($r, $g, $b) = explode(',', $color['fill']['low']);
 			$fill = imagecolorallocate($im, $r, $g, $b);
 		}
 
 		$quota_width = $quota / 100 * $width;
-		imagefilledrectangle($im, $border, 0, $quota, $height-2*$border, $fill);
+		imagefilledrectangle($im, $border, 0, $quota_width, $height-2*$border, $fill);
 
 		$string = $quota . '%';
 		$mid    = floor(($width-(strlen($string)*imagefontwidth($font)))/2)+1;
-        // Print percent in black
+		// Print percent in black
 		imagestring($im, $font, $mid, $padding, $string, $text); 
 	}
 
 	header('Content-Type: image/gif');
-    
-    // @todo is harcoding GMT necessary?
-	header('Expires: ' . gmdate('D, d M Y H:i:s', mktime()+86400) . ' GMT');
-	header('Cache-Control: ');
-	header('Pragma: ');
+
+	// cache for 1 hour
+	$maxage = 3600;
+	header('Expires: ' . gmdate('D, d M Y H:i:s', time()+$maxage). ' GMT');
+	header('Cache-Control: max-age=' . $maxage);
 	
 	imagegif($im);
 	imagedestroy($im);
 }
 
-genQuota($used, $quota, $width, $height);
+if ($width > 1 && $height > 1) {
+	genQuota($used, $quota, $width, $height);  
+}
+else {
+	header("HTTP/1.0 404 Not Found");
+}
+
 exit;
-?>
\ No newline at end of file
+?>

pgp6AZ8JanfO8.pgp
Description: PGP signature