Page 1 of 3

Yep, Another login script

Posted: Sun Mar 29, 2009 8:54 pm
by singleString
Now my knowledge of php security is not very vast, and I'm just beginning with php, so I was just thought if you could take a quick look over my login script for a website I'm working on even though you are probably tired to death of them.

The script is just to log a user in so they can access a control panel for the site.

It's pretty self explanatory I think, but if not you can ask about anything not that I didn't think you would.

Little comments are better than no comments.

Thanks in advance. (I hope)

Code: Select all

<?php
//set counter for number of times a person has logged in unsuccesfully
//loginLimit();
 
//sets mysql variables
include("/srv/www/mysql.php");
loginSql();
 
$result = $GLOBALS["result"];
$connection = $GLOBALS["connection"];
 
//checks if user is remembered
if(isset($_COOKIE["username"]) && isset($_COOKIE["password"]) && isset($_COOKIE["group"]))
{
    $user = $_COOKIE["username"];
    $pass = $_COOKIE["password"];
    $group = $_COOKIE["group"];
    $checkSql = "SELECT * FROM `login` WHERE user='$user' AND `password`='$pass' AND `group`='$group'";
    print $checkSql;
    print "<br />";
    $checkResult = mysql_query($checkSql, $connection);
    
    if($checkResult == TRUE)
    {
        session_name("CP");
        session_start();
        $_SESSION['user'] = $_COOKIE["username"];
        $_SESSION['pass'] = $_COOKIE["password"];
        $_SESSION['group'] = $_COOKIE["group"];
        session_write_close();
                            
        writeLog();
        header('Location: CPanel.php');
        exit();
    }
    else
    {
        header('Location: index.php');
        exit();
    }
}
 
//checks if username and or password was input  
if ($_REQUEST["username"] == "")
{
    if($_REQUEST["password"] == "")
    {
        writeLog();
        loginLimit();
        header('Location: index.php?nUoP=1');
        //print('No User Name or Password');
    }
    else
    {
        writeLog();
        loginLimit();
        header('Location: index.php?nU=1');
        //print('No User');
    }
}
else if ($_REQUEST["password"] == "")
{
    writeLog();
    loginLimit();
    header('Location: index.php?nP=1');
    //print('No Password');
}
else
{
    checkLogin();
}
 
//checks if username and password inputed were correct
function checkLogin()
{
    global $result;
    $user = $_REQUEST["username"];
    $totalRows = mysql_num_rows($result);
    $count = 0;
    
    while($login = mysql_fetch_row($result))
    {
        if($user == $login[0])
        {
            if (md5($_REQUEST["password"]) == "$login[1]")
                {
                    $pass = md5($_REQUEST["password"]);
                    
                    session_name("CP");
                    session_start();
                    $_SESSION['user'] = $user;
                    $_SESSION['pass'] = $login[1];
                    $_SESSION['group'] = $login[2];
                    session_write_close();
                    
                    if($_POST["remember"] == "YES")
                    {
                        setcookie("username", $user, time()+60*60*24*100);  
                        setcookie("password", $login[1], time()+60*60*24*100);
                        setcookie("group", $login[2], time()+60*60*24*100); 
                    }       
                                        
                    writeLog();
                    header('Location: CPanel.php');
                }
                else
                {
                    writeLog();
                    loginLimit();
                    header('Location: index.php?iP=1');
                }
        }
        else
        {
            $count++;
        }
        
        if($count == $totalRows)
        {
            writeLog();
            loginLimit();
            header('Location: index.php?iU=1');
        }
        else
        {
        }
    }
}
 
//writes login attempt to log
function writeLog()
{
    $connection = $GLOBALS["connection"];
    
    $user = $_REQUEST["username"];
    $date = date("l dS \of F Y h:i:s A");
    $ip = $_SERVER['REMOTE_ADDR'];
 
    $sql = "INSERT INTO `login log` VALUES ('$user', '$date', '$ip')";
    mysql_query($sql, $connection);
}
 
//set tries cookie
function loginLimit()
{
    if(isset($_REQUEST["tries"]))
    {
        $tries = $_REQUEST["tries"];
        $tries++;
    }
    else
    {
        $tries = 1;
    }
    setcookie("tries", $tries, time() + 500);
    
}
?>

Re: Yep, Another login script

Posted: Mon Mar 30, 2009 6:09 am
by kaisellgren

Code: Select all

include("/srv/www/mysql.php");
Since you are loading it directly, how about putting the file in /srv/ ?

Line 18 is vulnerable to SQL injections.

I think line 19 is for debugging purposes, but if you leave it there, you are vulnerable to XSS.

You should replace the line 23 with mysql_num_rows().

writeLog() is vulnerable to SQL injections.

Does CPanel.php make sure that the user is authenticated?

What is the point of the login limit if you do not call it prior to line 39?

The login tries can be altered easily.

You should not use $_REQUEST, use either $_POST or $_GET.

Do not use MD5. I recommend SHA-256, SHA-512 or Whirlpool.

Re: Yep, Another login script

Posted: Mon Mar 30, 2009 7:05 pm
by singleString
Regarding the SQL injections, how could I change it to make them make them uninjectable, and what is SQL injection?

The Cpanel.php page does check the user authentication.

Your right about the login limit, I'll take some time into seeing another way to do that another time.

Thank you for pointing out the use of $_REQUEST in here, I thought I had changed it early but it seems I hadn't.

Just out of curiosity, what is the difference between MD5 and the other you mentioned.

Re: Yep, Another login script

Posted: Tue Mar 31, 2009 4:37 am
by kaisellgren
singleString wrote:Regarding the SQL injections, how could I change it to make them make them uninjectable, and what is SQL injection?
I just don't understand why people are imcapable of googling... http://en.wikipedia.org/wiki/SQL_injection
singleString wrote:Just out of curiosity, what is the difference between MD5 and the other you mentioned.
Plenty of differences. See my blog entry about hashing. The main reason why not to use MD5 is safety.

Re: Yep, Another login script

Posted: Wed Apr 01, 2009 5:09 pm
by singleString
So, based on the article you kindly google for me, this should prevent the Sql Injection.

Correct?

Code: Select all

$illegalCharacters = array("\"", "\\", "/", "*", "'", "=", "-", "#", ";", "<", ">", "+", "%"), "$";
 
$user  = str_replace($illegalCharacters, "", $user);
$password = str_replace($illegalCharacters, "", $password);
$group = str_replace($illegalCharacters, "", $group);

Re: Yep, Another login script

Posted: Wed Apr 01, 2009 5:27 pm
by kaisellgren
Are you using MySQL? If so, just use the built-in function mysql(i)_real_escape_string().

Re: Yep, Another login script

Posted: Wed Apr 01, 2009 5:44 pm
by singleString
Yes I am, and was unaware of this function, thank you for pointing it out.

Re: Yep, Another login script

Posted: Wed Apr 08, 2009 10:58 am
by temidayo
kaisellgren wrote: You should not use $_REQUEST, use either $_POST or $_GET.
I want to believe that statement is about optimization rather than security.
Or is there a security risk attached to using $_REQUEST?

Re: Yep, Another login script

Posted: Wed Apr 08, 2009 11:17 am
by kaisellgren
temidayo wrote:I want to believe that statement is about optimization rather than security.
Or is there a security risk attached to using $_REQUEST?
It implies risks and in some cases they become serious.

For instance, let's say blogspot.com uses the following code:

Code: Select all

if ($_REQUEST['username'] != ... || $_REQUEST['password'] != ...)
{
 increase_failed_logins();
}
Everytime the user logins with wrong credentials, it increases a counter, which will forbid access to the site once it has reached 3 failed attempts.

Now, if I make my own blog at attacker.blogspot.com to set a cookie for blogspot.com with username=asd, then the user will automatically get banned and will never access the main site.

$_REQUEST involves $_GET, $_POST and $_COOKIES.

Whenever you accept unnecessary alternatives, you increase the number of risks in your application.

No, it has nothing to do with optimization.

Re: Yep, Another login script

Posted: Wed Apr 15, 2009 9:19 pm
by vivekanandanpcn8
For Login First you have to make use of SSl protocol and then for faster and more security have the session in th database which more reliable and used in te bussinedd critical application.more info http://www.gnudeveloper.com

Re: Yep, Another login script

Posted: Thu Apr 16, 2009 6:09 am
by kaisellgren
vivekanandanpcn8 wrote:For Login First you have to make use of SSl protocol and then for faster and more security have the session in th database which more reliable and used in te bussinedd critical application.more info http://www.gnudeveloper.com
To me it seems you do not know what you are talking about. Are you here just to advertise your new board?

Re: Yep, Another login script

Posted: Mon Apr 20, 2009 5:21 am
by temidayo
kaisellgren wrote:Whenever you accept unnecessary alternatives, you increase the number of risks in your application.
No, it has nothing to do with optimization.
That point is taken.
But that does not mean $_REQUEST should be avoided like a curse.
For instance, you may want to create APIs or a kind of interface on your website
and you do not want to worry either the data is submitted through POST or GET or COOKIES.

I still think one neat way of doing it is to use $_REQUEST instead of testing if it is GET or POST or otherwise

Re: Yep, Another login script

Posted: Mon Apr 20, 2009 6:35 am
by kaisellgren
temidayo wrote:But that does not mean $_REQUEST should be avoided like a curse.
The way I look at $_REQUEST is that it is like a disease. For sure you may be able to handle it, but why to risk?
temidayo wrote:For instance, you may want to create APIs or a kind of interface on your website and you do not want to worry either the data is submitted through POST or GET or COOKIES.
You must worry where the data comes from. Another thing is that your application must know to where it outputs the data. Data I/O is an important aspect of security.
temidayo wrote:I still think one neat way of doing it is to use $_REQUEST instead of testing if it is GET or POST or otherwise
If your application needs $_REQUEST to work, it is not designed well. POST is for data manipulation, GET is not. COOKIES are for "persistent" client side data "storage".

Re: Yep, Another login script

Posted: Mon Apr 20, 2009 12:33 pm
by temidayo
kaisellgren wrote: The way I look at $_REQUEST is that it is like a disease. For sure you may be able to handle it, but why to risk?
I don't know whether to agree or not :)
kaisellgren wrote: If your application needs $_REQUEST to work, it is not designed well. POST is for data manipulation, GET is not. COOKIES are for "persistent" client side data "storage".
You strike out $_REQUEST. Now you have something against $_GET :) . What is GET for, if not for 'data manipulation'?
When a website allows you to send parameters through url, how do you collect it? Is it not through $_GET or $_REQUEST?

Re: Yep, Another login script

Posted: Mon Apr 20, 2009 12:39 pm
by kaisellgren
temidayo wrote:You strike out $_REQUEST. Now you have something against $_GET :) . What is GET for, if not for 'data manipulation'?
When a website allows you to send parameters through url, how do you collect it? Is it not through $_GET or $_REQUEST?
I have nothing against GET.

http://www.w3.org/MarkUp/html-spec/html ... tml#SEC8.2

GET:
If the processing of a form is idempotent (i.e. it has no lasting observable effect on the state of the world), then the form method should be `GET'. Many database searches have no visible side-effects and make ideal applications of query forms.
POST:
If the service associated with the processing of a form has side effects (for example, modification of a database or subscription to a service), the method should be `POST'.