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 $libdirWhy 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 asideI 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: [EMAIL PROTECTED]
For additional commands, e-mail: [EMAIL PROTECTED]
To contact the list administrators, e-mail: [EMAIL PROTECTED]
