Re: Security Issues

At 08:48 26/07/2001, Rasmus Lerdorf wrote:

Just replace your if ($ok) with if (!empty($ok)), and you have a perfect exploitable code that produces no warnings. But in this case someone has gone to the trouble to figure out what empty() does. And generally they use empty() on things that come from the user anyway. I don't think I have ever seen people use empty() to check to see if they themselves have set an internal variable to something.

I have... And quite a lot use isset(), which is much more common and popular than empty(). My guess is that we're dealing with a fairly big number of users/scripts here, which are exploitable even though they're 'clean' scripts with no warnings. I'm all in favour of turning E_NOTICE's on (I was since day one and so was Andi, but back then, most people supported the 'loose' behavior of PHP/FI 2 as a default, and leave E_NOTICEs as an option you have to explicitly turn on).

Right, but I gave a more specific example - HTTPS, which doesn't always exist, and its very existence implies something. Not all variables are exploitable, perhaps even most aren't, but many are. Don't people use $SERVER_PORT for this? Would seem to make more sense. But sure, there may be a few variables like this.

Yes, it was just an example. I know at least one script in zend.com which uses HTTPS that I came across a while ago, so such cases definitely exist.

They're trivial to come up with; Pretty much any good code (E_NOTICE free) that doesn't take into account the fact that variables can pop out of nowhere into your global namespace is exploitable. I still think that turning on E_NOTICE by default is a good first step that we can take right away without waiting for a 4.1 and adding more emphasis to the documentation about these issues. With the E_NOTICE people at least get a warning and a line number and most apps will still work, they will just look very ugly forcing people to go and address the warnings. Turning off register_globals will simply break the applications with absolutely no hint as to why the application was broken and the end result will be that people simply turn register_globals back on to get things to work.

I actually think that turning E_NOTICE on is going to have a huge effect on a mind boggling number of scripts, probably on the same order of magnitude as setting register_globals to off (probably less, but not that much less). I think that unless we explain explicitly and vocally why we're making these changes (register_globals and/or error level), people will just reconfigure php.ini to the old settings - I don't think they'll start running after new E_NOTICE's they suddenly get after upgrading, unless they'd know they have a good reason to. I think that new PHP installations should come with register_globals set to off. Instead of php.ini-dist and php.ini-optimized, we should probably have php.ini-recommended, which would be the default installed file (assuming there's no php.ini to begin with), and php.ini-compat, which would be what php.ini-dist is today. No doubt, this would cause many (perhaps even most) PHP applications out there not to install out of the box, but I think that setting register_globals off is a pretty pressing matter, and without some pain (i.e., forcing app authors to update their apps to work with the track_vars arrays) we won't get anywhere. The way I think we should go about this is: (a) Improve the accessibility of the track vars array by shortening their names to $_GET[], $_POST[], etc. and possibly making them implicitly available inside functions. This can be done relatively quickly. (b) Get the word out that as of PHP 4.0.8, register_globals will be off, so that app authors will have a chance to fix their apps. (c) Release 4.0.7 with the new improved track vars, but with register_globals still set to on (d) Release 4.0.8 with register_globals set to off We can (and probably should) make the E_NOTICE change at the same time. Zeev