Re: [PHP-CVS] cvs: php-src /ext/session session.c

2007-06-16 Thread Stefan Esser
Stanislav Malyshev schrieb:
> That's nice. Could you now explain why you need these symbols in
> session IDs?
>
Even Zend Platform used ':' in session IDs not long ago. Maybe recent
versions of Zend Platform don't, but that is not the point.

The point is YOU DON'T KNOW how many people use one of these characters
in session IDs. YOU DON'T KNOW how many people use the PHP session
management but use the session_id() function to provide their OWN
session identifier. After all THAT is the reason for the session_id()
function.
Now without any warning you set a bunch of characters on a blacklist.
For no real reasons, just to NOT encode them when sending them out in
the cookie.

Face it this will break backward compatibility and even if only one
single person would be affected by this, this BC break is completely
unecessary, because by encoding the ID it is possible to support all the
other characters.

And especially when the session id is something home generated that
directly comes out of base64 encoding it is very likely that it ends in
the character = which is forbidden by your blacklist.

OHH yeah and your invalid reason that these characters are forbidden in
cookie values is NONSENSE. First of all the Netscape Cookie 0 format
(which is the one used by PHP) clearly says that everything except
whitespace and semicolon is allowed and secondly just LOOK at the
cookies you have in your browser.
: = () ;  these characters are used everywhere. A good start are your
*.google.com cookies they contain nearly all of these chars.

Stefan Esser

> Stefan Esser wrote:
>> sesserSat Jun 16 07:47:46 2007 UTC
>>
>>   Modified files:  /php-src/ext/sessionsession.c
>>   Log:
>>   Fix attribute injection security bug correctly by URL encoding
>> session   name and session value. (in future maybe encode
>> path/domain, too)
>> Remove backward compatibility breaking blacklist of characters.
>>
>> http://cvs.php.net/viewvc.cgi/php-src/ext/session/session.c?r1=1.472&r2=1.473&diff_format=u
>>
>> Index: php-src/ext/session/session.c
>> diff -u php-src/ext/session/session.c:1.472
>> php-src/ext/session/session.c:1.473
>> --- php-src/ext/session/session.c:1.472Fri Jun 15 22:42:43 2007
>> +++ php-src/ext/session/session.cSat Jun 16 07:47:46 2007
>> @@ -17,7 +17,7 @@
>>
>> +--+
>>   */
>>  
>> -/* $Id: session.c,v 1.472 2007/06/15 22:42:43 stas Exp $ */
>> +/* $Id: session.c,v 1.473 2007/06/16 07:47:46 sesser Exp $ */
>>  
>>  #ifdef HAVE_CONFIG_H
>>  #include "config.h"
>> @@ -398,7 +398,7 @@
>>  int vallen;
>>  
>>  /* check session name for invalid characters */
>> -if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\()@,;:[]?={}&%")) {
>> +if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {
>>  efree(PS(id));
>>  PS(id) = NULL;
>>  }
>> @@ -1069,6 +1069,7 @@
>>  {
>>  smart_str ncookie = {0};
>>  char *date_fmt = NULL;
>> +char *e_session_name, *e_id;
>>  
>>  if (SG(headers_sent)) {
>>  char *output_start_filename =
>> php_output_get_start_filename(TSRMLS_C);
>> @@ -1082,11 +1083,18 @@
>>  }   
>>  return;
>>  }
>> +   
>> +/* URL encode session_name and id because they might be user
>> supplied */
>> +e_session_name = php_url_encode(PS(session_name),
>> strlen(PS(session_name)), NULL);
>> +e_id = php_url_encode(PS(id), strlen(PS(id)), NULL);
>>  
>>  smart_str_appends(&ncookie, COOKIE_SET_COOKIE);
>> -smart_str_appends(&ncookie, PS(session_name));
>> +smart_str_appends(&ncookie, e_session_name);
>>  smart_str_appendc(&ncookie, '=');
>> -smart_str_appends(&ncookie, PS(id));
>> +smart_str_appends(&ncookie, e_id);
>> +   
>> +efree(e_session_name);
>> +efree(e_id);
>> 
>>  if (PS(cookie_lifetime) > 0) {
>>  struct timeval tv;
>>
>

-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-CVS] cvs: php-src /ext/session session.c

2007-06-16 Thread Stanislav Malyshev
That's nice. Could you now explain why you need these symbols in session 
IDs?


Stefan Esser wrote:

sesser  Sat Jun 16 07:47:46 2007 UTC

  Modified files:  
/php-src/ext/session	session.c 
  Log:
  Fix attribute injection security bug correctly by URL encoding session 
  name and session value. (in future maybe encode path/domain, too)
  
  Remove backward compatibility breaking blacklist of characters.
  
  
http://cvs.php.net/viewvc.cgi/php-src/ext/session/session.c?r1=1.472&r2=1.473&diff_format=u

Index: php-src/ext/session/session.c
diff -u php-src/ext/session/session.c:1.472 php-src/ext/session/session.c:1.473
--- php-src/ext/session/session.c:1.472 Fri Jun 15 22:42:43 2007
+++ php-src/ext/session/session.c   Sat Jun 16 07:47:46 2007
@@ -17,7 +17,7 @@
+--+
  */
 
-/* $Id: session.c,v 1.472 2007/06/15 22:42:43 stas Exp $ */

+/* $Id: session.c,v 1.473 2007/06/16 07:47:46 sesser Exp $ */
 
 #ifdef HAVE_CONFIG_H

 #include "config.h"
@@ -398,7 +398,7 @@
int vallen;
 
 	/* check session name for invalid characters */

-   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\()@,;:[]?={}&%")) {
+   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {
efree(PS(id));
PS(id) = NULL;
}
@@ -1069,6 +1069,7 @@
 {
smart_str ncookie = {0};
char *date_fmt = NULL;
+   char *e_session_name, *e_id;
 
 	if (SG(headers_sent)) {

char *output_start_filename = 
php_output_get_start_filename(TSRMLS_C);
@@ -1082,11 +1083,18 @@
}   
return;
}
+   
+   /* URL encode session_name and id because they might be user supplied */
+   e_session_name = php_url_encode(PS(session_name), 
strlen(PS(session_name)), NULL);
+   e_id = php_url_encode(PS(id), strlen(PS(id)), NULL);
 
 	smart_str_appends(&ncookie, COOKIE_SET_COOKIE);

-   smart_str_appends(&ncookie, PS(session_name));
+   smart_str_appends(&ncookie, e_session_name);
smart_str_appendc(&ncookie, '=');
-   smart_str_appends(&ncookie, PS(id));
+   smart_str_appends(&ncookie, e_id);
+   
+   efree(e_session_name);
+   efree(e_id);

if (PS(cookie_lifetime) > 0) {
struct timeval tv;



--
Stanislav Malyshev, Zend Products Engineer
[EMAIL PROTECTED]  http://www.zend.com/

--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-CVS] cvs: php-src(PHP_4_4) /ext/session session.c

2007-06-16 Thread Stefan Esser
sesser  Sat Jun 16 07:48:23 2007 UTC

  Modified files:  (Branch: PHP_4_4)
/php-src/ext/sessionsession.c 
  Log:
  MFH
  
  
http://cvs.php.net/viewvc.cgi/php-src/ext/session/session.c?r1=1.336.2.53.2.19&r2=1.336.2.53.2.20&diff_format=u
Index: php-src/ext/session/session.c
diff -u php-src/ext/session/session.c:1.336.2.53.2.19 
php-src/ext/session/session.c:1.336.2.53.2.20
--- php-src/ext/session/session.c:1.336.2.53.2.19   Fri Jun 15 22:45:25 2007
+++ php-src/ext/session/session.c   Sat Jun 16 07:48:23 2007
@@ -17,7 +17,7 @@
+--+
  */
 
-/* $Id: session.c,v 1.336.2.53.2.19 2007/06/15 22:45:25 stas Exp $ */
+/* $Id: session.c,v 1.336.2.53.2.20 2007/06/16 07:48:23 sesser Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -666,7 +666,7 @@
int vallen;
 
/* check session name for invalid characters */
-   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\()@,;:[]?={}&%")) {
+   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {
efree(PS(id));
PS(id) = NULL;
}
@@ -918,6 +918,7 @@
 {
smart_str ncookie = {0};
char *date_fmt = NULL;
+   char *e_session_name, *e_id;
 
if (SG(headers_sent)) {
char *output_start_filename = 
php_get_output_start_filename(TSRMLS_C);
@@ -931,11 +932,18 @@
}   
return;
}
+   
+   /* URL encode session_name and id because they might be user supplied */
+   e_session_name = php_url_encode(PS(session_name), 
strlen(PS(session_name)), NULL);
+   e_id = php_url_encode(PS(id), strlen(PS(id)), NULL);
 
smart_str_appends(&ncookie, COOKIE_SET_COOKIE);
-   smart_str_appends(&ncookie, PS(session_name));
+   smart_str_appends(&ncookie, e_session_name);
smart_str_appendc(&ncookie, '=');
-   smart_str_appends(&ncookie, PS(id));
+   smart_str_appends(&ncookie, e_id);
+   
+   efree(e_session_name);
+   efree(e_id);

if (PS(cookie_lifetime) > 0) {
struct timeval tv;

-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-CVS] cvs: php-src(PHP_5_2) /ext/session session.c

2007-06-16 Thread Stefan Esser
sesser  Sat Jun 16 07:48:07 2007 UTC

  Modified files:  (Branch: PHP_5_2)
/php-src/ext/sessionsession.c 
  Log:
  MFH
  
  
http://cvs.php.net/viewvc.cgi/php-src/ext/session/session.c?r1=1.417.2.8.2.36&r2=1.417.2.8.2.37&diff_format=u
Index: php-src/ext/session/session.c
diff -u php-src/ext/session/session.c:1.417.2.8.2.36 
php-src/ext/session/session.c:1.417.2.8.2.37
--- php-src/ext/session/session.c:1.417.2.8.2.36Fri Jun 15 22:40:00 2007
+++ php-src/ext/session/session.c   Sat Jun 16 07:48:07 2007
@@ -17,7 +17,7 @@
+--+
  */
 
-/* $Id: session.c,v 1.417.2.8.2.36 2007/06/15 22:40:00 stas Exp $ */
+/* $Id: session.c,v 1.417.2.8.2.37 2007/06/16 07:48:07 sesser Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -807,7 +807,7 @@
int vallen;
 
/* check session name for invalid characters */
-   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\()@,;:[]?={}&%")) {
+   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {
efree(PS(id));
PS(id) = NULL;
}
@@ -1080,6 +1080,7 @@
 {
smart_str ncookie = {0};
char *date_fmt = NULL;
+   char *e_session_name, *e_id;
 
if (SG(headers_sent)) {
char *output_start_filename = 
php_get_output_start_filename(TSRMLS_C);
@@ -1093,11 +1094,18 @@
}   
return;
}
+   
+   /* URL encode session_name and id because they might be user supplied */
+   e_session_name = php_url_encode(PS(session_name), 
strlen(PS(session_name)), NULL);
+   e_id = php_url_encode(PS(id), strlen(PS(id)), NULL);
 
smart_str_appends(&ncookie, COOKIE_SET_COOKIE);
-   smart_str_appends(&ncookie, PS(session_name));
+   smart_str_appends(&ncookie, e_session_name);
smart_str_appendc(&ncookie, '=');
-   smart_str_appends(&ncookie, PS(id));
+   smart_str_appends(&ncookie, e_id);
+   
+   efree(e_session_name);
+   efree(e_id);

if (PS(cookie_lifetime) > 0) {
struct timeval tv;

-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



[PHP-CVS] cvs: php-src /ext/session session.c

2007-06-16 Thread Stefan Esser
sesser  Sat Jun 16 07:47:46 2007 UTC

  Modified files:  
/php-src/ext/sessionsession.c 
  Log:
  Fix attribute injection security bug correctly by URL encoding session 
  name and session value. (in future maybe encode path/domain, too)
  
  Remove backward compatibility breaking blacklist of characters.
  
  
http://cvs.php.net/viewvc.cgi/php-src/ext/session/session.c?r1=1.472&r2=1.473&diff_format=u
Index: php-src/ext/session/session.c
diff -u php-src/ext/session/session.c:1.472 php-src/ext/session/session.c:1.473
--- php-src/ext/session/session.c:1.472 Fri Jun 15 22:42:43 2007
+++ php-src/ext/session/session.c   Sat Jun 16 07:47:46 2007
@@ -17,7 +17,7 @@
+--+
  */
 
-/* $Id: session.c,v 1.472 2007/06/15 22:42:43 stas Exp $ */
+/* $Id: session.c,v 1.473 2007/06/16 07:47:46 sesser Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include "config.h"
@@ -398,7 +398,7 @@
int vallen;
 
/* check session name for invalid characters */
-   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\()@,;:[]?={}&%")) {
+   if (PS(id) && strpbrk(PS(id), "\r\n\t <>'\"\\")) {
efree(PS(id));
PS(id) = NULL;
}
@@ -1069,6 +1069,7 @@
 {
smart_str ncookie = {0};
char *date_fmt = NULL;
+   char *e_session_name, *e_id;
 
if (SG(headers_sent)) {
char *output_start_filename = 
php_output_get_start_filename(TSRMLS_C);
@@ -1082,11 +1083,18 @@
}   
return;
}
+   
+   /* URL encode session_name and id because they might be user supplied */
+   e_session_name = php_url_encode(PS(session_name), 
strlen(PS(session_name)), NULL);
+   e_id = php_url_encode(PS(id), strlen(PS(id)), NULL);
 
smart_str_appends(&ncookie, COOKIE_SET_COOKIE);
-   smart_str_appends(&ncookie, PS(session_name));
+   smart_str_appends(&ncookie, e_session_name);
smart_str_appendc(&ncookie, '=');
-   smart_str_appends(&ncookie, PS(id));
+   smart_str_appends(&ncookie, e_id);
+   
+   efree(e_session_name);
+   efree(e_id);

if (PS(cookie_lifetime) > 0) {
struct timeval tv;

-- 
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php



Re: [PHP-CVS] cvs: php-src(PHP_5_2) /ext/gd/libgd gd.c

2007-06-16 Thread Pierre

On 6/15/07, Stanislav Malyshev <[EMAIL PROTECTED]> wrote:

> + pts = (char *) ecalloc(im->sy * im->sx, sizeof(char));

I don't see any overflow checks around, are you sure it's safe?


Checks are done in gdImageCreate*

--Pierre

--
PHP CVS Mailing List (http://www.php.net/)
To unsubscribe, visit: http://www.php.net/unsub.php