|
|
Posted by Steve on 02/26/06 17:18
> I writed the following code for my loggin script:
> <?php
> $username = htmlentities($_POST['login']);
> $password = htmlentities($_POST['passw']);
> $submited = $_POST['submited']; // hidden variable in login form (value =
> 'yes')
> $browser_f = $_POST['browser']; // browser type and ver from login form
> $browser_l = $_SERVER['HTTP_USER_AGENT'];
> $time = time() - $_POST['time']; // time difference
> $page = "../admin/admin.php"; // protected page
>
> if (!isset($_REQUEST['login']) || ($submited != 'yes') || ($time > 180)) {
> echo "\n Intruder alert!\n";
> exit(); }
> if ((!$username) || (!$password)) {
> echo "\n No data enetered!\n";
> exit(); }
> if(strstr($username,"<") || strstr($password,"<") || strstr($username,">")
> || strstr($password,">") || strstr($username,"script") ||
> strstr($password,"java")) {
> echo "\n No codding please!\n";
> exit(); }
> if ($username == 'username' && $password == 'password' && $browser_f ==
> $browser_l) {
> session_register("logged_in");
> header("Location:$page");
> exit(); }
> else {
> echo "\n Access denied!\n";
> exit(); }
> ?>
>
> Is this safe enougt or I should put some other checks in it?
Just some random comments...
Keep the username and password in a separate PHP file out of the web
tree and use include() to import them.
You don't need to encode htmlentities in $username and $password or
check them for tags or script, unless you are plan to display or store
them which you don't here.
Don't print any information about why a login fails, just fail. You can
add a link to a "request for help" page if you like, but don't leak
useful information about your validation methods.
Don't test for $_POST *and* $_REQUEST as you are offering alternative
means of providing the authorisation values. Stick to $_POST.
What stops me from requesting ../admin/admin.php directly? Do you test
for the session there?
Finally, don't do this piecemeal: do your homework...
<http://phpsec.org/php-security-guide.pdf> (Warning: links to PDF)
---
Steve
Navigation:
[Reply to this message]
|