Re: Security Issues

UNSUBSCRIBE ME PLEASE!!!!!!!!!!!!!! Zeev Suraski schrieb: > At 06:01 28/07/2001, Phil Driscoll wrote: > >Apologies in advance to Zeev for arguing on this one, but be assured I firmly > >believe that your solution would be to the detriment of PHP and a better > >solution is possible :) > > > >On Saturday 28 July 2001 12:44, Zeev Suraski wrote: > > > > > > NO!! :) Saying people would stop using PHP (or won't get started) because > > > of this change is a gross exaggeration. IMHO, the one and only issue at > > > hand here is downwards compatibility, and not usability or ease of use, not > > > even a bit. > > > >Sorry Zeev - the answer is YES. I and no doubt thousands of others will turn > >register_globals on because it gives much more readable code, much less > >typing and does not IMHO add one jot to the security of my applications. > > I have no doubt that thousands would turn it back on. I can't do anything > about it, and as I said numerous times in numerous metaphors, I'm quite > alright with that. I also have no doubt that the alternative ways we'll > provide would result in a firm 'NO', and definitely not a 'YES'. I > can't > imagine a single person stopping to use PHP because the default changed > (why the heck not just change it back, isn't it easier than learning a > whole new alternative platform?). I also can't imagine people avoiding PHP > because variables are accessed using $_FORM['foo'], instead of > $foo. People are not *THAT* dumb or lazy. Discussing this issue in the > OSCon, Rasmus claimed that right now he can teach PHP to a monkey in 3 > hours, and he wouldn't want to be limited only to smart Gorillas in the > future. I firmly believe that if a monkey can figure out $foo, > $_FORM['foo'] is not going to be the showstopper. > > > > That's not going to find half, or a quarter, or whatever of the problems, > > > since PHP has tools to cleanly handle undefined variables - namely isset() > > > and empty(). They, or at least isset(), are quite popular. > > > >I always use something like: > > > >if(!isset($Thing) /*and possibly some range checking*/)) > > $Thing="sensible default"; > > > >In no way is > > > >if(!isset(_GET['Thing']) /*and possibly some range checking*/)) > >{ > > $Thing="sensible default"; > >} > >else > >{ > > $Thing=(_GET['Thing']); > >} > > > >any more secure (nor would it be if I wrote "sensible default" back to _GET. > > Any programming language gives you a long enough rope to hang you. I argue > that PHP gives you a hanging loop and places it on your neck, leaving you > only a couple of steps away from breaking it. Your example clearly states > a piece of code where at the end, you have $Thing defined. In the 2nd > example, it's clear beyond a reasonable doubt that it may come from user > input, because it's written explicitly. However, if we turn $Thing back to > $Auth, and add it in context: > > /* part 1 */ > if (sth) { > $Auth = 1; > } > /* end part 1 */ > ...a few dozen lines of code... > /* part 2 */ > if (isset($Auth)) { > ... do something ... > } else { > ... do something else ... > } > /* end of part 2 */ > > In this case, having written part 1, it's not clear *at all* that Auth may > be coming from user input. Should it be? Yeah. But as I've been > repeating myself again and again in the last few days, as the advisory > said, effectively it is NOT. Even Brian, which opposes the > register_globals=off thing, found quirks in his code that he did not > anticipate because of the state of mind PHP puts you in. In part 2, you're > completely vulnerable to attacks. If part 1 had to be explicit, the danger > would have been a hell of a lot clearer. > So yes, you can hang yourself still, you're just a few feet farther away > from the hanging loop now, and it comes with a warning sign. > > >Anyway, to check my sanity i have reread the security advisory which I first > >read on the day it was published, and I am even more conviced now that > >register globals=off has the tiniest of effects for gpc variables wheras > >E_NOTICE has a massive effect. > > > >Here are the examples from the advisory: > > > >------------------ > > <?php > > if ($pass === "hello") //= corrected to === > > $auth = 1; > > ... > > if ($auth == 1) > > echo "some important information"; > > ?> > > > >replace $pass with _GET['pass'] and the code is > >equally insecure. Turn E_NOTICE on and the novice programmer will get a > >warning message for the unset $pass. > > No it is not. $auth can no longer come from user input, so it's > safer. $pass being unset is not much of a problem. > > >------------------ > > <?php > > if (!($fd = fopen("$filename", "r")) > > echo("Could not open file: $filename<BR>\n"); > > ?> > > > >replace $filename with _GET['filename'] and this lunatic piece of code > >remains a lunatic piece of code. If $filename is not meant to be coming from > >the outside world then with E_NOTICE on there would be a warning message for > >the unset filename. > > How is that security? And what if I pass filename=""? > > >------------------ > > <?php > > include($libdir . "/languages.php"); > > ?> > >Ok, with register_globals=off then $libdir could not be directly overwritten > >from outside (unless there was some code which made that happen) however > >E_NOTICE would generate a warning for an unset $libdir > > Why would libdir be unset? The issue here is that under some > circumstances, you can override variables. Different things happen in > different paths of your logic, so testing is a nightmare if you just rely > on the E_NOTICE feature. > > >------------------ > >File upload. > >If you don't use the new functions you are potentially stuffed with register > >globals on or off. > > Having written the new functions, I don't recall why you'd be stuffed with > register_globals=off. As far as I recall, the entire problem was a direct > result of register_globals=on, and not enough track_vars functionality > available at the time to write secure code. It was a while ago, so I could > be wrong, though. > > >As an aside - we could have a php.ini directive which could 'break' code > >which did not use the new functions - if we save an uploaded file with one > >name, but set the $userfile_name to something else, and only rename the > >original to $userfile_name after a call to is_uploaded_file or > >move_uploaded_file. > >------------------ > > <?php > > if (file_exists($theme)) // Checks the file exists on the local system (no > >remote files) > > include("$theme"); > > ?> > > > >This seems to be the same as the fopen($filename... example above. > > Again, I'm not exactly sure what the security risk here is, but I just woke > up :) > > >------------------ > >In libdir/loadlanguage.php: > > <?php > > ... > > > > include("$langDir/$userLang"); > > ?> > > > >include files called out of context. The solution is to configure your web > >server, or put the include files outside the webroot so that they cannot be > >run out of context. > > > >In this example, typically $langdir should have been set inside the > >application or a configuration file, but $userLang will typically have come > >from the user either with this request, or previously and stored in a > >database in the users profile. > > > >So under the register_globals = off scheme, we would often end up with > >something along the lines of: > > > ><?php > > ... > > > > include("$langDir/_GET['userLang']"); > > ?> > > > >where $langDir is unset. We can all see how secure that is :) > > But that's running towards the hanging loop from a mile away, wrapping it > quickly around your neck and pushing the lever. You can't do much about > suicidals :) > > >------------------ > >Session files. > > > >I am happy to concede that there should be a php.imi directive which stops > >variables which can more or less be trusted, such as env and session from > >entering the global namespace, so that if you want to read them you have to > >look in the correct place. > > Cool :) > > >------------------ > > > >In conclusion I would urge those who want to set register_globals=off to > >reconsider. A better solution is required. The better solution involves some, > >all, or more of: > >E_WARNING > >more granularity to the register_globals directive > >the file upload changes I mentioned as an aside > > I think your examples were very good reasons why in fact register_globals > should be set to off by default. I think that in the state of mind you're > in, you're no really accounting for the state of mind that most PHP > developers are, and thus, making a mistake by assuming that turning > register_globals off is not going to have a far reaching security improving > effect. Even if people just go over their code and explicitly declare > which variables are coming from the user space, even that alone is going to > help them audit their apps to a much higher level of security than it is today. > > Zeev > > -- > PHP Development Mailing List <http://www.php.net/> > To unsubscribe, e-mail: php-dev-unsubscribe@lists.php.net > For additional commands, e-mail: php-dev-help@lists.php.net > To contact the list administrators, e-mail: php-list-admin@lists.php.net