Re: [PHP-CVS] cvs: php-src(PHP_5_2) / NEWS /ext/filter filter.c /ext/filter/tests bug39763.phpt

2006-12-09 Thread Ilia Alshanetsky


On 8-Dec-06, at 2:05 PM, Stefan Esser wrote:


c) Support Cookies correctly...

Very simple: In some earlier version php_register_variable_ex was
changed to handle cookies different from other variables: Cookies with
the same name will get dropped after the first is registered.
In ext filter the raw variables still have this behaviour but the
filtered variables behave different...


The cookie discrepancy is now fixed, interestingly enough due to  
another bug (look inside latest patch in php_variables.c) it was  
actually working correctly in browsers that put a space after the ;  
token in to the Cookie header.


Ilia Alshanetsky

--
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) / NEWS /ext/filter filter.c /ext/filter/tests bug39763.phpt

2006-12-08 Thread Ilia Alshanetsky
iliaa   Fri Dec  8 17:50:04 2006 UTC

  Added files: (Branch: PHP_5_2)
/php-src/ext/filter/tests   bug39763.phpt 

  Modified files:  
/php-srcNEWS 
/php-src/ext/filter filter.c 
  Log:
  Fixed bug #39763 (magic quotes are applied twice by ext/filter in
  parse_str())
  
  
  
http://cvs.php.net/viewvc.cgi/php-src/NEWS?r1=1.2027.2.547.2.412r2=1.2027.2.547.2.413diff_format=u
Index: php-src/NEWS
diff -u php-src/NEWS:1.2027.2.547.2.412 php-src/NEWS:1.2027.2.547.2.413
--- php-src/NEWS:1.2027.2.547.2.412 Fri Dec  8 17:11:42 2006
+++ php-src/NEWSFri Dec  8 17:50:03 2006
@@ -50,6 +50,8 @@
 - Fixed wrong signature initialization in imagepng (Takeshi Abe)
 - Added optimization for imageline with horizontal and vertial lines (Pierre)
 - Fixed bug #39775 (Indirect modification ... message is not shown). (Dmitry)
+- Fixed bug #39763 (magic quotes are applied twice by ext/filter in
+  parse_str()). (Ilia) 
 - Fixed bug #39754 (Some POSIX extension functions not thread safe).
   (Ilia, wharmby at uk dot ibm dot com)
 - Fixed bug #39724 (Broken build due to spl/filter usage of pcre extension).
http://cvs.php.net/viewvc.cgi/php-src/ext/filter/filter.c?r1=1.52.2.34r2=1.52.2.35diff_format=u
Index: php-src/ext/filter/filter.c
diff -u php-src/ext/filter/filter.c:1.52.2.34 
php-src/ext/filter/filter.c:1.52.2.35
--- php-src/ext/filter/filter.c:1.52.2.34   Fri Dec  8 17:04:01 2006
+++ php-src/ext/filter/filter.c Fri Dec  8 17:50:04 2006
@@ -19,7 +19,7 @@
   +--+
 */
 
-/* $Id: filter.c,v 1.52.2.34 2006/12/08 17:04:01 tony2001 Exp $ */
+/* $Id: filter.c,v 1.52.2.35 2006/12/08 17:50:04 iliaa Exp $ */
 
 #ifdef HAVE_CONFIG_H
 #include config.h
@@ -275,7 +275,7 @@
 {
php_info_print_table_start();
php_info_print_table_row( 2, Input Validation and Filtering, 
enabled );
-   php_info_print_table_row( 2, Revision, $Revision: 1.52.2.34 $);
+   php_info_print_table_row( 2, Revision, $Revision: 1.52.2.35 $);
php_info_print_table_end();
 
DISPLAY_INI_ENTRIES();
@@ -397,7 +397,7 @@
Z_STRVAL(new_var) = estrndup(*val, val_len);
INIT_PZVAL(tmp_new_var);
php_zval_filter(tmp_new_var, IF_G(default_filter), 
IF_G(default_filter_flags), NULL, NULL/*charset*/, 0 TSRMLS_CC);
-   } else if (PG(magic_quotes_gpc)) {
+   } else if (PG(magic_quotes_gpc)  !retval) { /* for 
PARSE_STRING php_register_variable_safe() will do the addslashes() */
Z_STRVAL(new_var) = php_addslashes(*val, 
Z_STRLEN(new_var), Z_STRLEN(new_var), 0 TSRMLS_CC);
} else {
Z_STRVAL(new_var) = estrndup(*val, val_len);

http://cvs.php.net/viewvc.cgi/php-src/ext/filter/tests/bug39763.phpt?view=markuprev=1.1
Index: php-src/ext/filter/tests/bug39763.phpt
+++ php-src/ext/filter/tests/bug39763.phpt

-- 
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) / NEWS /ext/filter filter.c /ext/filter/tests bug39763.phpt

2006-12-08 Thread Stefan Esser

   php_zval_filter(tmp_new_var, IF_G(default_filter), 
 IF_G(default_filter_flags), NULL, NULL/*charset*/, 0 TSRMLS_CC);
 - } else if (PG(magic_quotes_gpc)) {
 + } else if (PG(magic_quotes_gpc)  !retval) { /* for 
 PARSE_STRING php_register_variable_safe() will do the addslashes() */
   Z_STRVAL(new_var) = php_addslashes(*val, 
 Z_STRLEN(new_var), Z_STRLEN(new_var), 0 TSRMLS_CC);
   
This comment is wrong. It is not php_register_variable_safe() but
ext/filter that adds the magic_quotes.

And Antony's previous commit never fixed anything, it just broke
magic_quotes_gpc and completely disabled it, introducing possible SQL
injection vulnerabilities in tons of scripts...

BTW: When will ext/filter be rewritten to
a) support daisy chaining
b) does not register the variables itself but actually work as filters
were supposed to do.
c) Support Cookies correctly...

Stefan Esser

-- 
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) / NEWS /ext/filter filter.c /ext/filter/tests bug39763.phpt

2006-12-08 Thread Ilia Alshanetsky

On 8-Dec-06, at 1:26 PM, Stefan Esser wrote:



 			php_zval_filter(tmp_new_var, IF_G(default_filter), IF_G 
(default_filter_flags), NULL, NULL/*charset*/, 0 TSRMLS_CC);

-   } else if (PG(magic_quotes_gpc)) {
+		} else if (PG(magic_quotes_gpc)  !retval) { /* for  
PARSE_STRING php_register_variable_safe() will do the addslashes() */
 			Z_STRVAL(new_var) = php_addslashes(*val, Z_STRLEN(new_var),  
Z_STRLEN(new_var), 0 TSRMLS_CC);



This comment is wrong. It is not php_register_variable_safe() but
ext/filter that adds the magic_quotes.


This is where you are incorrect, take a look at  
php_register_variable_safe() line #51. The reason we don't apply  
magic quotes for PARSE_STRING (retval == 1) is because the return  
value of the function in this instance is 1, which causes  
php_register_variable_safe() to be executed on the returned value.



a) support daisy chaining


Can you not provide your own hooks and have them call the stock  
filter functions?



b) does not register the variables itself but actually work as filters
were supposed to do.


I think it makes the API simpler, but it would not be major change to  
make if there was a really good reason to do so.



c) Support Cookies correctly...


Can you elaborate in terms of what is missing in cookie support?

Ilia Alshanetsky

--
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) / NEWS /ext/filter filter.c /ext/filter/tests bug39763.phpt

2006-12-08 Thread Ilia Alshanetsky


On 8-Dec-06, at 2:05 PM, Stefan Esser wrote:


a) support daisy chaining


Can you not provide your own hooks and have them call the stock  
filter

functions?

I (my extensions) have no problem with that. I just wonder why PHP got
input filter hooks in one version and in a few versions later they are
not useable anymore, because ext/filter grabs them.


Don't get me wrong its not a bad idea to have daisy chaining, its  
just that there was no need for it expressed before so it was never  
done.


b) does not register the variables itself but actually work as  
filters

were supposed to do.

I think it makes the API simpler, but it would not be major change to
make if there was a really good reason to do so.

The reason is simple: The filter extension abuses the input filtering
hooks. The variables are no longer registered at one single place but
several codepaths lead to different results.
The bug you fixed with your commit is the best example: If ext/filter
would not try to register the variables itself, this bug would never
have happened.


There are a few reasons why it is doing it, foremost of which is the  
attempt to reduce the amount of memory utilized while registering  
variables. The last thing we want to do is allocate, re-allocate and  
so on every input parameter. I suspect there is a way to achieve a  
solution that would prevent duplication of data, which we still have  
right now with the RAW filter.



c) Support Cookies correctly...

Very simple: In some earlier version php_register_variable_ex was
changed to handle cookies different from other variables: Cookies with
the same name will get dropped after the first is registered.
In ext filter the raw variables still have this behaviour but the
filtered variables behave different...


I need to look into that, I recall adding checks for cookie hierarchy  
based on paths into core PHP, but I have not played with the  
equivalent filter code yet.


Ilia Alshanetsky

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