CVE-2012-0807: Improper Restriction of Operations within the Bounds of a Memory Buffer (CWE-119)
value: Stack-based buffer overflow in the suhosin_encrypt_single_cookie function in the transparent cookie-encryption feature in the Suhosin extension before 0.9.33 for PHP, when suhosin.cookie.encrypt and suhosin.multiheader are enabled, might allow remote attackers to execute arbitrary code via a long string that is used in a Set-Cookie HTTP header.
CWE description: The product performs operations on a memory buffer, but it reads from or writes to a memory location outside the buffer's intended boundary. This may result in read or write operations on unexpected memory locations that could be linked to other variables, data structures, or internal program data.
Published: 2012-01-27T00:55Z
Repository: https://github.com/stefanesser/suhosin
Language: Other
Files changed: Changelog, header.c
Commit date: 2012-01-14 09:44:17 +0100
Commit message: Fixed stack based buffer overflow in transparent cookie encryption (see separate advisory)

Diff: diff --git a/Changelog b/Changelog
index 6c18103..8c2a8a4 100644
--- a/Changelog
+++ b/Changelog
@@ -1,7 +1,8 @@
 2012-01-11 - 0.9.33-dev
 
+    - Fixed stack based buffer overflow in transparent cookie encryption (see separate advisory)
     - Fixed that disabling HTTP response splitting protection also disabled NUL byte protection in HTTP headers
     - Removed crypt() support - because not used for PHP >= 5.3.0 anyway
 
 2010-07-23 - 0.9.32.1
 
diff --git a/header.c b/header.c
index 368e085..74d4ad9 100644
--- a/header.c
+++ b/header.c
@@ -38,86 +38,60 @@ static int (*orig_header_handler)(sapi_header_struct *sapi_header, sapi_header_o
 static int (*orig_header_handler)(sapi_header_struct *sapi_header, sapi_headers_struct *sapi_headers TSRMLS_DC) = NULL;
 #endif
 
 char *suhosin_encrypt_single_cookie(char *name, int name_len, char *value, int value_len, char *key TSRMLS_DC)
 {
-	char buffer[4096];
-    char buffer2[4096];
-	char *buf = buffer, *buf2 = buffer2, *d, *d_url;
-    int l;
-
-	if (name_len > sizeof(buffer)-2) {
-		buf = estrndup(name, name_len);
-	} else {
-		memcpy(buf, name, name_len);
-		buf[name_len] = 0;
-	}
+	char *buf, *buf2, *d, *d_url;
+	int l;
+
+	buf = estrndup(name, name_len);
+	
 	
 	name_len = php_url_decode(buf, name_len);
-    normalize_varname(buf);
-    name_len = strlen(buf);
+	normalize_varname(buf);
+	name_len = strlen(buf);
 	
 	if (SUHOSIN_G(cookie_plainlist)) {
 		if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {
 encrypt_return_plain:
-			if (buf != buffer) {
-				efree(buf);
-			}
+			efree(buf);
 			return estrndup(value, value_len);
 		}
 	} else if (SUHOSIN_G(cookie_cryptlist)) {
 		if (!zend_hash_exists(SUHOSIN_G(cookie_cryptlist), buf, name_len+1)) {
 			goto encrypt_return_plain;
 		}
 	}
 	
-	if (strlen(value) <= sizeof(buffer2)-2) {
-		memcpy(buf2, value, value_len);
-		buf2[value_len] = 0;
-	} else {
-		buf2 = estrndup(value, value_len);
-	}
+	buf2 = estrndup(value, value_len);
 	
 	value_len = php_url_decode(buf2, value_len);
 	
 	d = suhosin_encrypt_string(buf2, value_len, buf, name_len, key TSRMLS_CC);
 	d_url = php_url_encode(d, strlen(d), &l);
 	efree(d);
-    if (buf != buffer) {
-		efree(buf);
-	}
-    if (buf2 != buffer2) {
-		efree(buf2);
-	}
+	efree(buf);
+	efree(buf2);
 	return d_url;
 }
 
 char *suhosin_decrypt_single_cookie(char *name, int name_len, char *value, int value_len, char *key, char **where TSRMLS_DC)
 {
-	char buffer[4096];
-    char buffer2[4096];
     int o_name_len = name_len;
-	char *buf = buffer, *buf2 = buffer2, *d, *d_url;
+	char *buf, *buf2, *d, *d_url;
 	int l;
 
-	if (name_len > sizeof(buffer)-2) {
-		buf = estrndup(name, name_len);
-	} else {
-		memcpy(buf, name, name_len);
-		buf[name_len] = 0;
-	}
-	
+	buf = estrndup(name, name_len);
+		
 	name_len = php_url_decode(buf, name_len);
-    normalize_varname(buf);
-    name_len = strlen(buf);
+	normalize_varname(buf);
+	name_len = strlen(buf);
 	
 	if (SUHOSIN_G(cookie_plainlist)) {
 		if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {
 decrypt_return_plain:
-			if (buf != buffer) {
-				efree(buf);
-			}
+			efree(buf);
             memcpy(*where, name, o_name_len);
             *where += o_name_len;
             **where = '='; *where +=1;
 	        memcpy(*where, value, value_len);
 	        *where += value_len;
@@ -128,16 +102,11 @@ decrypt_return_plain:
 			goto decrypt_return_plain;
 		}
 	}
 	
 	
-	if (strlen(value) <= sizeof(buffer2)-2) {
-		memcpy(buf2, value, value_len);
-		buf2[value_len] = 0;
-	} else {
-		buf2 = estrndup(value, value_len);
-	}
+	buf2 = estrndup(value, value_len);
 	
 	value_len = php_url_decode(buf2, value_len);
 	
 	d = suhosin_decrypt_string(buf2, value_len, buf, name_len, key, &l, SUHOSIN_G(cookie_checkraddr) TSRMLS_CC);
     if (d == NULL) {
@@ -150,16 +119,12 @@ decrypt_return_plain:
     **where = '=';*where += 1;
 	memcpy(*where, d_url, l);
 	*where += l;
 	efree(d_url);
 skip_cookie:
-	if (buf != buffer) {
-		efree(buf);
-	}
-	if (buf2 != buffer2) {
-		efree(buf2);
-	}
+	efree(buf);
+	efree(buf2);
 	return *where;
 }
 
 /* {{{ suhosin_cookie_decryptor
  */

Vulnerable code: 	char buffer[4096];
    char buffer2[4096];
	char *buf = buffer, *buf2 = buffer2, *d, *d_url;
    int l;

	if (name_len > sizeof(buffer)-2) {
		buf = estrndup(name, name_len);
	} else {
		memcpy(buf, name, name_len);
		buf[name_len] = 0;
	}
    normalize_varname(buf);
    name_len = strlen(buf);
			if (buf != buffer) {
				efree(buf);
			}
	if (strlen(value) <= sizeof(buffer2)-2) {
		memcpy(buf2, value, value_len);
		buf2[value_len] = 0;
	} else {
		buf2 = estrndup(value, value_len);
	}
    if (buf != buffer) {
		efree(buf);
	}
    if (buf2 != buffer2) {
		efree(buf2);
	}
	char buffer[4096];
    char buffer2[4096];
	char *buf = buffer, *buf2 = buffer2, *d, *d_url;
	if (name_len > sizeof(buffer)-2) {
		buf = estrndup(name, name_len);
	} else {
		memcpy(buf, name, name_len);
		buf[name_len] = 0;
	}
	
    normalize_varname(buf);
    name_len = strlen(buf);
			if (buf != buffer) {
				efree(buf);
			}
	if (strlen(value) <= sizeof(buffer2)-2) {
		memcpy(buf2, value, value_len);
		buf2[value_len] = 0;
	} else {
		buf2 = estrndup(value, value_len);
	}
	if (buf != buffer) {
		efree(buf);
	}
	if (buf2 != buffer2) {
		efree(buf2);
	}
Fixed code:     - Fixed stack based buffer overflow in transparent cookie encryption (see separate advisory)
	char *buf, *buf2, *d, *d_url;
	int l;

	buf = estrndup(name, name_len);
	
	normalize_varname(buf);
	name_len = strlen(buf);
			efree(buf);
	buf2 = estrndup(value, value_len);
	efree(buf);
	efree(buf2);
	char *buf, *buf2, *d, *d_url;
	buf = estrndup(name, name_len);
		
	normalize_varname(buf);
	name_len = strlen(buf);
			efree(buf);
	buf2 = estrndup(value, value_len);
	efree(buf);
	efree(buf2);