recode my php login? :)

[Derbel]

alrite, i've made this php login with sessions and have been told that its very rubbishly coded and very insecure, any chance anyone could recode it for me? make it faster, and more secure. oh and less to do? ;]

thanks in advance.




<?php

include('php/config.php');

session_start();

if(!session_is_registered(username)){

if(isset($_POST['username']) && isset($_POST['password'])){
$username = ($_POST['username']);
$password = ($_POST['password']);
$remember_me = $_POST['_login_remember_me'];

if(empty($username) || empty($password)){
$login_error = "Please do not leave any fields blank.";
} else {
$sql = mysql_query("SELECT id FROM users WHERE name = '".$username."' AND password = '".$password."' LIMIT 1") or die(mysql_error());
$rows = mysql_num_rows($sql);
if($rows < 1){
$login_error = "Invalid username or password.";
} else {
$userdata = mysql_fetch_assoc($sql);
$userid = $userdata['id'];
$check = mysql_query("SELECT * FROM users_bans WHERE userid = '".$userid."' OR ipaddress = '".$remote_ip."' LIMIT 1") or die(mysql_error());
$is_banned = mysql_num_rows($check);
if($is_banned < 1){
$_SESSION['username'] = $username;
$_SESSION['password'] = $password;
if($remember_me == "true"){
setcookie("remember", "remember", time()+60*60*24*100, "/");
setcookie("rusername", $_SESSION['username'], time()+60*60*24*100, "/");
setcookie("rpassword", sha1("zomq".$_SESSION['password']), time()+60*60*24*100, "/");
}
$sql3 = mysql_query("UPDATE users SET lastvisit = '".$date_full."' WHERE name = '".$username."'") or die(mysql_error());
header("location:security_check.php"); exit;
} else {
$bandata = mysql_fetch_assoc($check);
$reason = $bandata['descr'];
$expire = $bandata['date_expire'];

$xbits = explode(" ", $expire);
$xtime = explode(":", $xbits[1]);
$xdate = explode("-", $xbits[0]);

$stamp_now = mktime(date('H'),date('i'),date('s'),$today,$month ,$year);
$stamp_expire = mktime($xtime[0], $xtime[1], $xtime[2], $xdate[0], $xdate[1], $xdate[2]);

if($stamp_now < $stamp_expire){
$login_error = "You have been banned! The reason for this ban is \"".$reason."\". The ban will expire at ".$expire.".";
} else { // ban expired
mysql_query("DELETE FROM users_bans WHERE userid = '".$userid."' OR ipaddress = '".$remote_ip."' LIMIT 1") or die(mysql_error());
$_SESSION['username'] = $username;
$_SESSION['password'] = $password;
if($remember_me == "true"){
setcookie("remember", "remember", time()+60*60*24*100, "/");
setcookie("rusername", $_SESSION['username'], time()+60*60*24*100, "/");
setcookie("rpassword", sha1("zomq".$_SESSION['password']), time()+60*60*24*100, "/");
}
$sql3 = mysql_query("UPDATE users SET lastvisit = '".$date_full."' WHERE name = '".$username."'") or die(mysql_error());
header("location:security_check.php"); exit;
}
}
}
}
}

if(isset($_GET['error'])){
$errorno = $_GET['error'];
if($errorno == 1){
$login_error = "Invalid username or password.";
} elseif($errorno == 2){
$login_error = "Invalid username or password.";
} elseif(isset($_GET['ageLimit']) && $_GET['ageLimit'] == "true"){
$login_error = "You are too young to register.";
}
}


?>

<?php
if(isset($login_error)){
echo "\n<div class=\"action-error flash-message\">\n <div class=\"rounded\">\n <ul>\n <li>".$login_error."</li>\n </ul>\n </div>\n</div>\n";
}
?>


<form action="login.php?do=process_login" method="post" class="login-habblet">
<input tabindex="1" type="text" class="login-field" name="username" id="login-username" value="Username" />
<input tabindex="2" type="password" class="login-field" name="password" id="login-password" value="Password" />
<input type="submit" value="Sign in" class="submit" id="login-submit-button"/>
<a href="#" id="login-submit-new-button" class="new-button" style="float: left; margin-left: 0;display:none"><b style="padding-left: 10px; padding-right: 7px; width: 55px">Sign in</b><i></i></a>
</li>
<li class="no-label">
<input tabindex="3" type="checkbox" name="_login_remember_me" id="login-remember-me" value="true"/>
<label for="login-remember-me">Remember me</label>
</li>
<li class="no-label">
<a href="register.php" class="login-register-link"><span>Register</span></a>
</li>
<li class="no-label">
<a href="forgot.php" id="forgot-password"><span>Forgot</span></a>
</li>
</ul>
</form>

<?php
include('footer.php');

} else {
}
?>

Thread Closed by Flisker (Forum Moderator): Closed because it was bumped.

[Ryzie]

What's wrong with it?

[Anzrew]

Research SQL Injection, as it is possible at the moment.

If you dont allow certain characters in usernames like ' or " that would make it far harder to SQL inject and you could filter those out using preg_match or even str_replace. If you search SQL Injection on google and you can also look up get_magic_quotes_gpc and mysql_real_escape_string (PHP Functions) to help prevent sql injection.

Next time when you post your code you may want to leave out a few small bits of information like:
setcookie("rpassword", sha1("zomq".$_SESSION['password']), time()+60*60*24*100, "/");
As now we all know you encrypt passwords using sha1 with zomq infront of it, should someone be able to get hold of the encrypted data, and manage to find the original password then they need only take off zomq from infront of it to login as the user. Its extemely unlikly, but better safe than sorry. Just replace the line you omited when posting with a comment like // Line ommited for security, setcookie() is used.

Hope this helps.

[Excellent1]

No offense but I highly doubt you coded this as the coding there is quite clean and properly done. Also you wouldn't ask someone to re-code it if you can already do it..

[kreechin]

No offense but I highly doubt you coded this as the coding there is quite clean and properly done. Also you wouldn't ask someone to re-code it if you can already do it..


Notice the class="login-habblet"
it's a habbo based login, either for a retro or other habbo based login.
I'm guessing it's from a retro CMS login that's insecure.

[HotelUser]

Notice the class="login-habblet"
it's a habbo based login, either for a retro or other habbo based login.
I'm guessing it's from a retro CMS login that's insecure.

And that he didn't code. The script he posted was taken from holocms, see:
http://svn.assembla.com/svn/holocms3/holo/index.php

tut tut.. -rep for lieing.

[Jackboy]

I've recoded it for you.


<?php
session_start();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
?>

[HotelUser]

I've recoded it for you.


<?php
session_start();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
die();die();die();die();die();die();die();
?>

Job well done :eusa_clap.

[Excellent2]

<?php

echo "Oops, failure";

exit();

?>

[Dentafrice]

And that he didn't code. The script he posted was taken from holocms, see:
http://svn.assembla.com/svn/holocms3/holo/index.php

tut tut.. -rep for lieing.

Why bump and oldddd thread?



<?php

echo "Oops, failure";

exit();

?>

Why don't you just do it the easy and better way, :\



<?php
exit( "Oops, failure." );
?>

[Excellent2]

Why bump and oldddd thread?



Why don't you just do it the easy and better way, :\



<?php
exit( "Oops, failure." );
?>
Because that is my preference :)

[Dentafrice]

Because that is my preference :)
Why? Explain that.. it's stupid.

[Decode]

<?php

echo "Oops, failure";

exit();

?>

You can put strings and functions inside the exit/die functions so you could just do

die("Dead");

[Dentafrice]

Why use die? It's not going to stop immediately, it still has to go through cleanup procedures.

If you want it to immediately stop, exit() is your man.

Edited by Flisker (Forum Moderator): Posts merged due to forum lag.

[Protege]

OmG DOUBLE POST, dat lyk turns me on Caleb bbi

There isnt a perfence in echoin' before exiting, its pure stupidity.

its lyk that guy who made a func called "delete" and put in unlink inside.

[HotelUser]

Why bump and oldddd thread?
[/php]
Because I can and it's free..


Why use die? It's not going to stop immediately, it still has to go through cleanup procedures.

If you want it to immediately stop, exit() is your man.

Erm...is it relevant? It's how he likes to do it.

[Excellent2]

Why? Explain that.. it's stupid.It's just how I do it?

[Dentafrice]

Because I can and it's free..



Erm...is it relevant? It's how he likes to do it.


It's just how I do it?

Yeah, but you shouldn't do that..?

PHP has to output it, then exit (while checking for a message inside of it), when you could easily just save the process of outputting it (and storing it to the buffer) and just do them both at once.

[HotelUser]

Yeah, but you shouldn't do that..?

PHP has to output it, then exit (while checking for a message inside of it), when you could easily just save the process of outputting it (and storing it to the buffer) and just do them both at once.

No offense but once again, it's irrelevant, and you're just trying to sound like a know-it-all. I don't think he was being 100% serious when he said that either.....

[Dentafrice]

No offense but once again, it's irrelevant, and you're just trying to sound like a know-it-all. I don't think he was being 100% serious when he said that either.....
Yet I'm not a "no-it-all", when someone else agreed with me?

It's stupid, and I'm pointing it out.

Don't like it, get over it? That's the way it works here.

You don't have a clue what we are talking about anyway, so don't try to get involved.

[HotelUser]

Yet I'm not a "no-it-all", when someone else agreed with me?

It's stupid, and I'm pointing it out.

Don't like it, get over it? That's the way it works here.

You don't have a clue what we are talking about anyway, so don't try to get involved.

Once again your obnoxious comment make is appear as if you're trying to sound superior, "you don't even know what we're talking about...don't like it get over it....". If he wants to do something his way it's not your place to tell him differently. Get over it.


<?php
$denta = "It's stupid and I'm pointing it out.";
$denta = str_replace("It's", "I'm", $denta);
echo $denta;
?>Did you used to have the account Dentapoop or something like that?

[Dentafrice]

Once again your obnoxious comment make is appear as if you're trying to sound superior, "you don't even know what we're talking about...don't like it get over it....". If he wants to do something his way it's not your place to tell him differently. Get over it.



It's the ******* design and development forum, inside of Coding.. it's about coding..

He posts **** code, we're going to tell him, get the over it? It's not your place to tell me not to




<?php
$denta = "It's stupid and I'm pointing it out.";
$denta = str_replace("It's", "I'm", $denta);
echo $denta;
?>Did you used to have the account Dentapoop or something like that?


All that's going to do is point out that you're stupid?

Oh, the Dentapoop thing.. LOL. Insult book again?

[HotelUser]

He posts **** code, we're going to tell him, get the over it? It's not your place to tell me not to
It's not your place to tell him how to code, either. You're not his instructor, nor are you in supervisor. He didn't ask for help :rolleyes:.




All that's going to do is point out that you're stupid?

Oh, the Dentapoop thing.. LOL. Insult book again?

No, the dentapoop thing was a genuine question. I was talking with a friend on MSN, and he told me (and this is quite true) that he knew someone Dentapoop, who was a knowitall coder he hired to do a script for him, and he ended up arguing with Dentapoop and telling him off, "putting him in his place" he said. I just presumed it was you..Dentapoop, Dentafrice.



All that's going to do is point out that you're stupid?
Erm...according to the code, it's pointing out that you are stupid ;).

It's stupid, and I'm pointing it out.

[Dentafrice]

It's not your place to tell him how to code, either. You're not his instructor, nor are you in supervisor. He didn't ask for help :rolleyes:.

I'm not his instructor, nor his supervisor, but if I see someone doing something stupid.. of course I'm going to say something. It's a ******* coding forum.


No, the dentapoop thing was a genuine question. I was talking with a friend on MSN, and he told me (and this is quite true) that he knew someone Dentapoop, who was a knowitall coder he hired to do a script for him, and he ended up arguing with Dentapoop and telling him off, "putting him in his place" he said. I just presumed it was you..Dentapoop, Dentafrice.


Yeah, it's not me.




Erm...according to the code, it's pointing out that you are stupid ;).

No, it's saying you're stupid, hence with I'm..? If you wanted to point it at me.. you would have said Dentafrice, not I'm.