Fixed stack based buffer overflow in transparent cookie encryption (see separate advisory)

author: Stefan Esser 2012-01-14 09:44:17 +0100
committer: Stefan Esser 2012-01-14 09:44:17 +0100
commit: 73b1968ee30f6d9d2dae497544b910e68e114bfa (patch)
tree: ac6c8a3757dcd8a8622bf706a190a4a4f4d13bd5 /header.c
parent: f6453621b786a13e8be17fb1a6ee04601383f9d4 (diff)
1 files changed, 19 insertions, 54 deletions
diff --git a/header.c b/header.c
index 368e085..74d4ad9 100644
--- a/header.c
+++ b/header.c
@@ -40,28 +40,20 @@ static int (*orig_header_handler)(sapi_header_struct *sapi_header, sapi_headers_
 char *suhosin_encrypt_single_cookie(char *name, int name_len, char *value, int value_len, char *key TSRMLS_DC)
 {
-        char buffer[4096];
+        char *buf, *buf2, *d, *d_url;
-    char buffer2[4096];
+        int l;
-        char *buf = buffer, *buf2 = buffer2, *d, *d_url;
-    int l;
-        if (name_len > sizeof(buffer)-2) {
+        buf = estrndup(name, name_len);
-                buf = estrndup(name, name_len);
+        
-        } else {
-                memcpy(buf, name, name_len);
-                buf[name_len] = 0;
-        }
        
        name_len = php_url_decode(buf, name_len);
-    normalize_varname(buf);
+        normalize_varname(buf);
-    name_len = strlen(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)) {
@@ -70,52 +62,34 @@ encrypt_return_plain:
                }
        }
        
-        if (strlen(value) <= sizeof(buffer2)-2) {
+        buf2 = estrndup(value, value_len);
-                memcpy(buf2, value, value_len);
-                buf2[value_len] = 0;
-        } else {
-                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);
-                efree(buf);
+        efree(buf2);
-        }
-    if (buf2 != buffer2) {
-                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);
-                buf = estrndup(name, name_len);
+                
-        } else {
-                memcpy(buf, name, name_len);
-                buf[name_len] = 0;
-        }
-        
        name_len = php_url_decode(buf, name_len);
-    normalize_varname(buf);
+        normalize_varname(buf);
-    name_len = strlen(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;
@@ -130,12 +104,7 @@ decrypt_return_plain:
        }
        
        
-        if (strlen(value) <= sizeof(buffer2)-2) {
+        buf2 = estrndup(value, value_len);
-                memcpy(buf2, value, value_len);
-                buf2[value_len] = 0;
-        } else {
-                buf2 = estrndup(value, value_len);
-        }
        
        value_len = php_url_decode(buf2, value_len);
        
@@ -152,12 +121,8 @@ decrypt_return_plain:
        *where += l;
        efree(d_url);
 skip_cookie:
-        if (buf != buffer) {
+        efree(buf);
-                efree(buf);
+        efree(buf2);
-        }
-        if (buf2 != buffer2) {
-                efree(buf2);
-        }
        return *where;
 }
author	Stefan Esser	2012-01-14 09:44:17 +0100
committer	Stefan Esser	2012-01-14 09:44:17 +0100
commit	73b1968ee30f6d9d2dae497544b910e68e114bfa (patch)
tree	ac6c8a3757dcd8a8622bf706a190a4a4f4d13bd5 /header.c
parent	f6453621b786a13e8be17fb1a6ee04601383f9d4 (diff)

diff --git a/header.c b/header.c index 368e085..74d4ad9 100644 --- a/header.c +++ b/header.c
@@ -40,28 +40,20 @@ static int (orig_header_handler)(sapi_header_struct sapi_header, sapi_headers_
40		40
41	char suhosin_encrypt_single_cookie(char name, int name_len, char value, int value_len, char key TSRMLS_DC)	41	char suhosin_encrypt_single_cookie(char name, int name_len, char value, int value_len, char key TSRMLS_DC)
42	{	42	{
43	char buffer[4096];	43	char buf, buf2, d, d_url;
44	char buffer2[4096];	44	int l;
45	char buf = buffer, buf2 = buffer2, d, d_url;
46	int l;
47		45
48	if (name_len > sizeof(buffer)-2) {	46	buf = estrndup(name, name_len);
49	buf = estrndup(name, name_len);	47
50	} else {
51	memcpy(buf, name, name_len);
52	buf[name_len] = 0;
53	}
54		48
55	name_len = php_url_decode(buf, name_len);	49	name_len = php_url_decode(buf, name_len);
56	normalize_varname(buf);	50	normalize_varname(buf);
57	name_len = strlen(buf);	51	name_len = strlen(buf);
58		52
59	if (SUHOSIN_G(cookie_plainlist)) {	53	if (SUHOSIN_G(cookie_plainlist)) {
60	if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {	54	if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {
61	encrypt_return_plain:	55	encrypt_return_plain:
62	if (buf != buffer) {	56	efree(buf);
63	efree(buf);
64	}
65	return estrndup(value, value_len);	57	return estrndup(value, value_len);
66	}	58	}
67	} else if (SUHOSIN_G(cookie_cryptlist)) {	59	} else if (SUHOSIN_G(cookie_cryptlist)) {
@@ -70,52 +62,34 @@ encrypt_return_plain:
70	}	62	}
71	}	63	}
72		64
73	if (strlen(value) <= sizeof(buffer2)-2) {	65	buf2 = estrndup(value, value_len);
74	memcpy(buf2, value, value_len);
75	buf2[value_len] = 0;
76	} else {
77	buf2 = estrndup(value, value_len);
78	}
79		66
80	value_len = php_url_decode(buf2, value_len);	67	value_len = php_url_decode(buf2, value_len);
81		68
82	d = suhosin_encrypt_string(buf2, value_len, buf, name_len, key TSRMLS_CC);	69	d = suhosin_encrypt_string(buf2, value_len, buf, name_len, key TSRMLS_CC);
83	d_url = php_url_encode(d, strlen(d), &l);	70	d_url = php_url_encode(d, strlen(d), &l);
84	efree(d);	71	efree(d);
85	if (buf != buffer) {	72	efree(buf);
86	efree(buf);	73	efree(buf2);
87	}
88	if (buf2 != buffer2) {
89	efree(buf2);
90	}
91	return d_url;	74	return d_url;
92	}	75	}
93		76
94	char suhosin_decrypt_single_cookie(char name, int name_len, char value, int value_len, char key, char **where TSRMLS_DC)	77	char suhosin_decrypt_single_cookie(char name, int name_len, char value, int value_len, char key, char **where TSRMLS_DC)
95	{	78	{
96	char buffer[4096];
97	char buffer2[4096];
98	int o_name_len = name_len;	79	int o_name_len = name_len;
99	char buf = buffer, buf2 = buffer2, d, d_url;	80	char buf, buf2, d, d_url;
100	int l;	81	int l;
101		82
102	if (name_len > sizeof(buffer)-2) {	83	buf = estrndup(name, name_len);
103	buf = estrndup(name, name_len);	84
104	} else {
105	memcpy(buf, name, name_len);
106	buf[name_len] = 0;
107	}
108
109	name_len = php_url_decode(buf, name_len);	85	name_len = php_url_decode(buf, name_len);
110	normalize_varname(buf);	86	normalize_varname(buf);
111	name_len = strlen(buf);	87	name_len = strlen(buf);
112		88
113	if (SUHOSIN_G(cookie_plainlist)) {	89	if (SUHOSIN_G(cookie_plainlist)) {
114	if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {	90	if (zend_hash_exists(SUHOSIN_G(cookie_plainlist), buf, name_len+1)) {
115	decrypt_return_plain:	91	decrypt_return_plain:
116	if (buf != buffer) {	92	efree(buf);
117	efree(buf);
118	}
119	memcpy(*where, name, o_name_len);	93	memcpy(*where, name, o_name_len);
120	*where += o_name_len;	94	*where += o_name_len;
121	*where = '='; where +=1;	95	*where = '='; where +=1;
@@ -130,12 +104,7 @@ decrypt_return_plain:
130	}	104	}
131		105
132		106
133	if (strlen(value) <= sizeof(buffer2)-2) {	107	buf2 = estrndup(value, value_len);
134	memcpy(buf2, value, value_len);
135	buf2[value_len] = 0;
136	} else {
137	buf2 = estrndup(value, value_len);
138	}
139		108
140	value_len = php_url_decode(buf2, value_len);	109	value_len = php_url_decode(buf2, value_len);
141		110
@@ -152,12 +121,8 @@ decrypt_return_plain:
152	*where += l;	121	*where += l;
153	efree(d_url);	122	efree(d_url);
154	skip_cookie:	123	skip_cookie:
155	if (buf != buffer) {	124	efree(buf);
156	efree(buf);	125	efree(buf2);
157	}
158	if (buf2 != buffer2) {
159	efree(buf2);
160	}
161	return *where;	126	return *where;
162	}	127	}
163		128