Re: [PHP-DEV] Segfault on mysqlnd stream cast
Hi Rasmus,
On 08/03/2013 07:51 PM, Rasmus Lerdorf wrote:
Hey Johannes, could you take a look at:
https://gist.github.com/anonymous/6143477
You can reproduce in 5.5 with:
sapi/cli/php ext/mysqli/tests/mysqli_poll_kill.php
main/streams/cast.c:306 is:
if (php_stream_is_filtered(stream)) {
but php_stream_is_filtered is just a macro that isn't expecting stream
to be null and you get the segfault there because it is trying to
dereference it. We could just add a null check, of course, but I think
the problem is in mysqlnd. mysqlnd_stream_array_to_fd_set() shouldn't be
trying to cast null streams.
-Rasmus
I just tried the combo PHP 5.5 (git) with MySQL 5.6 (13-dev) without
segfault. What's your setup?
Andrey
--
PHP Internals - PHP Runtime Development Mailing List
To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Session Id Collisions
Hi, On Mon, Aug 5, 2013 at 2:01 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Thank you for noticing crash. Data can be null, so the fix is OK. Removing the limitation that prohibits setting session ID is fine for me, too. Please, apply your patch. I thought we were in agreement about doing this properly in PHP.next? My arguments against this version of the patch still stand: On Thu, Jun 27, 2013 at 11:51 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: Hi Arpad, 2013/6/27 Arpad Ray array...@gmail.com I see the strict mode check is now implemented in the handlers and not session.c, presumably to keep ABI, but this means code is duplicated and the setting only actually works if the handler supports it. It's unfortunate timing since 5.5 has just gone, but I think it would make much more sense to have a new function in the structure (as in your original patch) and do this only in PHP.next. Having such an ini setting which quietly fails if using an unsupported handler is not good. I guess you could keep a whitelist of supported handlers but that's also obviously far from ideal. Thank you for comment. There are plenty of time before 5.6 release, I would like to fix ps_module API also. Current implementation requires ps_modules to access PS(id) to prevent rare case of crash. I'll try to implement a little better API for ps_modules. Arpad
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Mon, Aug 5, 2013 at 6:22 PM, Arpad Ray array...@gmail.com wrote: I thought we were in agreement about doing this properly in PHP.next? My arguments against this version of the patch still stand: We had long discussion and decided to apply maintained branches as security enhancement more than a year ago. We also planned to apply the patch into 5.3 originally, but 5.3 is security fix only now. Anyway, if users are resetting session id properly, they are protected against session adoption attacks. However, users are not protect their apps properly, then they are at the risk of session adoption. This fix is rather important for PHP, since there are many setups that share PHP with many apps. That's the reason why we decided to apply this patch into maintained branches. PHP web server admins should feel much safer than before with this feature. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Session Id Collisions
Hi Yasuo, On Mon, Aug 5, 2013 at 10:50 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Mon, Aug 5, 2013 at 6:22 PM, Arpad Ray array...@gmail.com wrote: I thought we were in agreement about doing this properly in PHP.next? My arguments against this version of the patch still stand: We had long discussion and decided to apply maintained branches as security enhancement more than a year ago. We also planned to apply the patch into 5.3 originally, but 5.3 is security fix only now. Anyway, if users are resetting session id properly, they are protected against session adoption attacks. However, users are not protect their apps properly, then they are at the risk of session adoption. This fix is rather important for PHP, since there are many setups that share PHP with many apps. That's the reason why we decided to apply this patch into maintained branches. PHP web server admins should feel much safer than before with this feature. I'm not against the idea in principle but still think having a security feature which just quietly fails if you're not using one of two modified handlers is really not good. I also think there's no great rush to add this, because as you say, it can be protected against in userland too. I would much rather have a robust, clean solution even if we have to wait until php.next for it. Arpad
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Mon, Aug 5, 2013 at 7:05 PM, Arpad Ray array...@gmail.com wrote: I'm not against the idea in principle but still think having a security feature which just quietly fails if you're not using one of two modified handlers is really not good. I also think there's no great rush to add this, because as you say, it can be protected against in userland too. I would much rather have a robust, clean solution even if we have to wait until php.next for it. As I wrote, we already had long discussion a year ago and decided to include maintained branches. This issue should be fixed 8 years ago. It's already too late to adopt IMHO. If you would like to know the details of risks, please refer to the RFC. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Session Id Collisions
Hi Yasuo, On Mon, Aug 5, 2013 at 11:10 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Mon, Aug 5, 2013 at 7:05 PM, Arpad Ray array...@gmail.com wrote: I'm not against the idea in principle but still think having a security feature which just quietly fails if you're not using one of two modified handlers is really not good. I also think there's no great rush to add this, because as you say, it can be protected against in userland too. I would much rather have a robust, clean solution even if we have to wait until php.next for it. As I wrote, we already had long discussion a year ago and decided to include maintained branches. Could you point me to where this was decided please? I don't see a vote or anything like a consensus in the previous threads. This issue should be fixed 8 years ago. It's already too late to adopt IMHO. If you would like to know the details of risks, please refer to the RFC. I understand the risk and your solution, I just think there's a better solution (more like your original patch if I recall correctly) and that we shouldn't rush to apply an inferior solution just because the issue is so old. Arpad
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Mon, Aug 5, 2013 at 7:26 PM, Arpad Ray array...@gmail.com wrote: Could you point me to where this was decided please? I don't see a vote or anything like a consensus in the previous threads. There isn't vote for this RFC since this is security. It's also a consensus. The main thread is this. It's been 2 years ago, not a year. Message-ID: CAGa2bXYDbDnTi1aF4Ts9oShyy= 2nkpatc34_+hg7zn0v+_m...@mail.gmail.com You will see a few other threads around. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Segfault on mysqlnd stream cast
On 08/05/2013 03:32 AM, Andrey Hristov wrote: I just tried the combo PHP 5.5 (git) with MySQL 5.6 (13-dev) without segfault. What's your setup? This is on my Ubuntu 13.04 laptop. mysql Ver 14.14 Distrib 5.5.32 with PHP 5.5 git just running make test. I get a core every time on that test in that same spot. -Rasmus -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] I want to work against Bug 44522 - Upload limit 2G
I have added a simple test case for Linux to verify it's basic functionality via the CLI server, and think it's ready to be merged to master to be able to test it within a wider audience. Objections, anyone? https://github.com/m6w6/php-src/compare/2Guploads Thank you Ralf! -- Regards, Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] I want to work against Bug 44522 - Upload limit 2G
On 5 August 2013 14:05, Michael Wallner m...@php.net wrote: I have added a simple test case for Linux to verify it's basic functionality via the CLI server, and think it's ready to be merged to master to be able to test it within a wider audience. Objections, anyone? https://github.com/m6w6/php-src/compare/2Guploads Johannes reminded me, that we don't have C99 stdint portable typedefs in a central PHP header file available yet. -- Regards, Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] I want to work against Bug 44522 - Upload limit 2G
Hi Mike, On Aug 5, 2013 3:58 PM, Michael Wallner m...@php.net wrote: On 5 August 2013 14:05, Michael Wallner m...@php.net wrote: I have added a simple test case for Linux to verify it's basic functionality via the CLI server, and think it's ready to be merged to master to be able to test it within a wider audience. Objections, anyone? https://github.com/m6w6/php-src/compare/2Guploads Johannes reminded me, that we don't have C99 stdint portable typedefs in a central PHP header file available yet. In 5.3+ it is easier. Other parts of php deals with that. M4 Macros are there for unices, on windows you have php_stdint (which transparently includes stdint with vc11 and up). -- Regards, Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Refactored magic methods
On Fri, Aug 2, 2013 at 9:55 PM, Levi Morrison morrison.l...@gmail.comwrote: If you have ideas or things to say, I'm listening. https://github.com/jpauli/php-src/compare/macroing Is there a reason you switched from names like `__toString` to `__tostring` (https://github.com/jpauli/php-src/compare/macroing#L2R12)? Yes because I use the same word for structure members access and for error message. That makes the refactoring takes place actually, if I had to change just one letter to capitalize it for just one method, that would add lots of code just for treating that particular case :-p Visually the macros clean things up a lot and better convey the intent behind the if / else if chain. However, I don't really like having a macro that works off of an `ELSE`, it just feels wrong. I don't have a better suggestion though. Yes elses inside macros are not a good idea, I pushed a different implementation having elses out of macro definitions. Julien.Pauli
[PHP-DEV] Re: PHP 5.5.2 RC1 is tagged
Julien Pauli in php.internals (Fri, 2 Aug 2013 10:05:00 +0200): Please test the release carefully and report any bugs. What is the best way to report things that are so small that opening an issue would be overkill? I have got some tiny remarks: - Typo in NEWS: OPcahce should be OPcache - Remove the reference to php_zip.dll in both php.ini-*'s - Add 'case 90' in ext/gd/libgd/gd_jpeg.c to avoid unknown for jpeg lib 9 Compiled all 4 variants with VC11 flawlessly. Jan -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Re: PHP 5.5.2 RC1 is tagged
On 8/5/13 8:12 AM, Jan Ehrhardt wrote: Julien Pauli in php.internals (Fri, 2 Aug 2013 10:05:00 +0200): Please test the release carefully and report any bugs. What is the best way to report things that are so small that opening an issue would be overkill? I have got some tiny remarks: - Typo in NEWS: OPcahce should be OPcache - Remove the reference to php_zip.dll in both php.ini-*'s - Add 'case 90' in ext/gd/libgd/gd_jpeg.c to avoid unknown for jpeg lib 9 Compiled all 4 variants with VC11 flawlessly. Jan Submit pull requests with patches, then nag until someone applies them. Then nag to get karma to make changes yourself. The OPCache typo is already fixed, I believe. http://git.php.net/?p=php-src.git;a=blob;f=NEWS;h=c01b43ed7bcda9d3c846df439cf32ead01c098d6;hb=refs/heads/PHP-5.5 The gd change might qualify for a bug report, so any background can be discussed. Chris -- christopher.jo...@oracle.com http://twitter.com/ghrd Free PHP Oracle book: http://www.oracle.com/technetwork/topics/php/underground-php-oracle-manual-098250.html -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Session Id Collisions
Hi Yasuo, On Mon, Aug 5, 2013 at 11:38 AM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Mon, Aug 5, 2013 at 7:26 PM, Arpad Ray array...@gmail.com wrote: Could you point me to where this was decided please? I don't see a vote or anything like a consensus in the previous threads. There isn't vote for this RFC since this is security. It's also a consensus. While this is a security concern, it's not a straightforward bug fix. When there's contention in how to fix it, I think there really should be a vote. I've read the other threads and I don't think has been any clear consensus about this issue and I, for one, am not happy to have what I feel is an inferior solution committed while it's still being discussed. To reiterate: this ini setting will quietly fail when using a handler which hasn't been patched, like memcached, or a custom handler. That's arguably worse than not having the setting at all since it could give people a false sense of security. Arpad
Re: [PHP-DEV] I want to work against Bug 44522 - Upload limit 2G
On 5 August 2013 16:19, Pierre Joye pierre@gmail.com wrote: Hi Mike, On Aug 5, 2013 3:58 PM, Michael Wallner m...@php.net wrote: On 5 August 2013 14:05, Michael Wallner m...@php.net wrote: I have added a simple test case for Linux to verify it's basic functionality via the CLI server, and think it's ready to be merged to master to be able to test it within a wider audience. Objections, anyone? https://github.com/m6w6/php-src/compare/2Guploads Johannes reminded me, that we don't have C99 stdint portable typedefs in a central PHP header file available yet. In 5.3+ it is easier. Other parts of php deals with that. M4 Macros are there for unices, on windows you have php_stdint (which transparently includes stdint with vc11 and up). Yes, Anatol gave quite a few comments on the PR, so I knew about win32/php_stdint.h, but I was tricked by my memory about a globally (PHP-wide) available portability header :) Thanks for the heads up though! -- Regards, Mike -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Tue, Aug 6, 2013 at 1:04 AM, Arpad Ray array...@gmail.com wrote: I think there really should be a vote. This means you don't really understand the true risk of this vulnerability. It allows permanent session ID fixation. This is CVE assigned vulnerability. Details are explained in the RFC and I don't want to explain fully in ML again. (We might discussed the details in secur...@php.net, but I think I wrote enough info) Please refer to the RFC. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Session Id Collisions
Hi Yasuo, On Mon, Aug 5, 2013 at 7:46 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Tue, Aug 6, 2013 at 1:04 AM, Arpad Ray array...@gmail.com wrote: I think there really should be a vote. This means you don't really understand the true risk of this vulnerability. It allows permanent session ID fixation. This is CVE assigned vulnerability. Details are explained in the RFC and I don't want to explain fully in ML again. (We might discussed the details in secur...@php.net, but I think I wrote enough info) Please refer to the RFC. I do really understand the risk... I'm saying there should be a vote not on whether or not to fix it, but on how to fix it. Ideally we can figure out something we're all happy with and don't need to vote, but while we so evidently disagree, I think we do. I'm not going to repeat my arguments against the committed solution yet again, but I really think we need a better one. Arpad
Re: [PHP-DEV] Session Id Collisions
Hi! I'm not going to repeat my arguments against the committed solution yet again, but I really think we need a better one. You are free to propose a better one. Since this topic is being discussed for almost 2 years and nobody came with anything better, as far as I know, I think it is reasonable on this stage to go with what we have. If you have something better that is not BC - you're welcome to make a pull against master, if you have something that is better and is BC - that's excellent, let's see it and if it works better, no problem getting it into 5.5. But as far as I see now, that is the only viable patch that we had during pretty long time, so sitting and waiting that something better comes along doesn't look like the best course of action. I think we waited enough so that anybody who had better solution had a chance to propose it and develop it, and given it is a real problem, I think at least solution that works for now is a good thing to have. -- Stanislav Malyshev, Software Architect SugarCRM: http://www.sugarcrm.com/ (408)454-6900 ext. 227 -- PHP Internals - PHP Runtime Development Mailing List To unsubscribe, visit: http://www.php.net/unsub.php
Re: [PHP-DEV] Session Id Collisions
Hi Stas, On Mon, Aug 5, 2013 at 8:23 PM, Stas Malyshev smalys...@sugarcrm.comwrote: I'm not going to repeat my arguments against the committed solution yet again, but I really think we need a better one. You are free to propose a better one. Since this topic is being discussed for almost 2 years and nobody came with anything better, as far as I know, I think it is reasonable on this stage to go with what we have. If you have something better that is not BC - you're welcome to make a pull against master, if you have something that is better and is BC - that's excellent, let's see it and if it works better, no problem getting it into 5.5. But as far as I see now, that is the only viable patch that we had during pretty long time, so sitting and waiting that something better comes along doesn't look like the best course of action. I think we waited enough so that anybody who had better solution had a chance to propose it and develop it, and given it is a real problem, I think at least solution that works for now is a good thing to have. As I've said I actually think Yasuo's original patch was a better approach, tackling the issue in session.c instead of leaving it up to all the handlers to implement. This would break BC but solves the major flaw of the ini setting working with some handlers and silently failing with others. I think it's also a cleaner approach in general. It's a real pity that missed the 5.5 boat. I'll have a think if there's a way to do this with BC, or at least to fail better. Arpad
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Tue, Aug 6, 2013 at 4:17 AM, Arpad Ray array...@gmail.com wrote: On Mon, Aug 5, 2013 at 7:46 PM, Yasuo Ohgaki yohg...@ohgaki.net wrote: On Tue, Aug 6, 2013 at 1:04 AM, Arpad Ray array...@gmail.com wrote: I think there really should be a vote. This means you don't really understand the true risk of this vulnerability. It allows permanent session ID fixation. This is CVE assigned vulnerability. Details are explained in the RFC and I don't want to explain fully in ML again. (We might discussed the details in secur...@php.net, but I think I wrote enough info) Please refer to the RFC. I do really understand the risk... It allows permanent session ID fixation due to browser implementations. To make matter worse than old days, recent browsers only send one outstanding cookie. This made attack detection impossible at server side. (i.e. bad countermeasure(?) took by browser developers) If you curious about this vulnerability fix still, please read the RFC and do a little experiments. I did the experiment 2 years ago (and even 10 years ago). I suppose things are not changed. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
Re: [PHP-DEV] Session Id Collisions
Hi Arpad, On Tue, Aug 6, 2013 at 4:33 AM, Arpad Ray array...@gmail.com wrote: Hi Stas, On Mon, Aug 5, 2013 at 8:23 PM, Stas Malyshev smalys...@sugarcrm.comwrote: I'm not going to repeat my arguments against the committed solution yet again, but I really think we need a better one. You are free to propose a better one. Since this topic is being discussed for almost 2 years and nobody came with anything better, as far as I know, I think it is reasonable on this stage to go with what we have. If you have something better that is not BC - you're welcome to make a pull against master, if you have something that is better and is BC - that's excellent, let's see it and if it works better, no problem getting it into 5.5. But as far as I see now, that is the only viable patch that we had during pretty long time, so sitting and waiting that something better comes along doesn't look like the best course of action. I think we waited enough so that anybody who had better solution had a chance to propose it and develop it, and given it is a real problem, I think at least solution that works for now is a good thing to have. As I've said I actually think Yasuo's original patch was a better approach, tackling the issue in session.c instead of leaving it up to all the handlers to implement. This would break BC but solves the major flaw of the ini setting working with some handlers and silently failing with others. I think it's also a cleaner approach in general. It's a real pity that missed the 5.5 boat. I'll have a think if there's a way to do this with BC, or at least to fail better. There is reason that we agreed that we do not have vote for this patch. I'll write up full description after the release, since not many people understand true risk of session adoption vulnerable session management. (I have to check browser behaviors again, though.) Please wait. Regards, -- Yasuo Ohgaki yohg...@ohgaki.net
