Create Account Site - Is this secure?

condoravenue · December 6, 2010

Proof of ownership: http://breckenridge-snow-report.com/test3/phpfreaks.txt.

My site: http://breckenridge-snow-report.com/test3/

Right now, I just want to know if there are any security problems with this site. It's not user friendly at the moment, but I think everything works.

I am aware that there are some valid emails (^{.%@gmail.com) that won't get validated the way I coded it, but I could care less about that cause no one has that sort of email.

Can people hack in and get other people's usernames and passwords?

Can robots create fake profiles?

Do you see any other problems?

I couldn't get sessions to work. That's supposed to be easy, but I couldn't figure it out and used cookies instead. Which should I use for this site, cookies or sessions?

Here is the code for the file that validates the info after someone creates an account.

<?php

$con = mysql_connect("localhost","brecke5","********");
mysql_select_db("brecke5_people", $con);

$pword = $_POST[password];
$cpword = $_POST[cpassword];
$phashed = sha1($pword);
$code = sha1(uniqid(rand()));
$username = $_POST[username];
$email = $_POST[email];
$dob = $_POST[dobMonth] . "/" . $_POST[dobDay] . "/" . $_POST[dobYear];

//Check username availability
$result = mysql_query("SELECT * FROM perm WHERE username='$username'");
while($row = mysql_fetch_array($result))
{
die("The username you have chosen is not available.");
}

//Check for duplicate email
$result = mysql_query("SELECT * FROM perm WHERE email='$email'");
while($row = mysql_fetch_array($result))
{
die("You already have an account with this email.");
}

//validate
if (strlen($pword) <  {die ("Password too short.");}
if ($pword != $cpword) {die ("Passwords didn't match.". $pword . $cpword);}
if (strlen($username) < 6) {die ("Username too short.");}
if (strlen($username) > 20) {die ("Username too long.");}
if ($_POST[dobMonth] == "" || $_POST[dobDay] == "" || $_POST[dobYear] == "") {die ("Date of birth invalid.");}
if ($email == "") {die ("Email invalid.");}
if (strlen($email) > 40) {die ("Email too long (> 40).");}
if (!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $email)) {die ("Email invalid.");}


mysql_query("INSERT INTO temp (code, username, email, password, dob)
VALUES ('$code', '$username', '$email', '$phashed', '$dob')");

mysql_close($con);

$to = $email;
$subject = "Sing Up";
$message = "Please visit the following link to confirm your account: http://breckenridge-snow-report.com/test3/confirm.php?passkey=$code";
$from = "My Site";
$headers = "From: $from";
mail($to,$subject,$message,$headers);
echo "Please check your email to confirm your account.";
?>

Here is the code for the page that validates when the user tries to change password.

<?php
if(!isset($_COOKIE['username'])) {header("location: login.php");}

$npword = $_POST["new"];
$npword2 = $_POST["new2"];
$nphashed = sha1($npword);
$cpword = $_POST["current"];
$cphashed = sha1($cpword);
$username = $_COOKIE['username'];


$con = mysql_connect("localhost","brecke5","********");
mysql_select_db("brecke5_people", $con);
$result = mysql_query("SELECT * FROM perm
WHERE username='$username'");
while($row = mysql_fetch_array($result)) {$current = $row['password'];}

//validate
if ($current != $cphashed) {die("Invalid old password.");}
if ($npword != $npword2) {die ("New passwords did not match.");}
if (strlen($npword) <  {die ("New password too short.");}

mysql_query("UPDATE perm SET password = '$nphashed'
WHERE username = '$username'");
mysql_close($con);
?>

<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01//EN" 
"http://www.w3.org/TR/html4/strict.dtd">
<html>
<head>
<title>Password Changed</title>
<meta http-equiv="Content-Type" content="text/html; charset=utf-8;">
</head>
<body>
<p>Your password has successfully been changed.<br><a href = "myaccount.php">My Account</a></p>
</body> 
</html>

If you need to see other code, let me know.

Thanks for all the help.

tomfmason · December 6, 2010

from the two snippets above it appears that some basic security fundamentals like sanitizing user input were missed . If I were you I would checkout Daniel's php security tutorial.

condoravenue · December 7, 2010

I made some changes. I tried to implement the mysql_real_escape_string() function for every everiable that ever goes into a query. I do not know if I did it right. Here is the code for the file that validates the info after someone creates an account.

<?php

$con = mysql_connect("localhost","name","**********");
mysql_select_db("brecke5_people", $con);
$passkey=mysql_real_escape_string($_GET['passkey']);
$result = mysql_query("SELECT * FROM temp WHERE code='$passkey'");
$keyfound = "false";
while($row = mysql_fetch_array($result))
  {
  $username = mysql_real_escape_string($row['username']);
  $email = mysql_real_escape_string($row['email']);
  $password = mysql_real_escape_string($row['password']);
  $keyfound = "true";
  }
if ($keyfound == "true") {
mysql_query("INSERT INTO perm (username, email, password)
VALUES ('$username', '$email', '$password')");}

mysql_query("DELETE FROM temp WHERE code='$passkey'");

mysql_close($con);
?>

I noticed that both $_POST[somevariable] and $_POST['somevariable'] work in most (all?) cases. Is there any reason I should be consistent?

Let me know if you find other security problems.

awjudd · December 7, 2010

$_POST[foo] and $_POST['foo'] both work because PHP will automatically upconvert single words of text that it doesn't know as a constant to a string.

That said, using $_POST[foo] is slower because not only does it cause notices to be thrown but it has to check for any definitions before it can assume that it is a string.

Sample:

<?php
    $arr = array ();

    for ( $x = 0; $x < 1000000; $x++ )
    {
        $arr [ foo ] = 'bar';
    }
?>

Time of execution:

$ time php test.php 

real    0m1.641s
user    0m1.424s
sys     0m0.044s

Versus:

<?php
    $arr = array ();

    for ( $x = 0; $x < 1000000; $x++ )
    {
        $arr [ 'foo' ] = 'bar';
    }
?>

Run Time:

$ time php test.php 

real    0m0.467s
user    0m0.292s
sys     0m0.052s

Yes, this is a highly exaggerated case but it is a decent example of how it is bad

~judda

ignace · December 7, 2010

Or a better example:

At some point in time you write:

if(isset($_POST[txt_username])) {
    // bla
}

At some later point in time you add translations to your code:

define('TXT_USERNAME', 'Type your username:', true);

And now your code breaks.

if(isset($_POST['Type your username:']))

condoravenue · December 9, 2010

I moved the site to http://breckenridge-snow-report.com/somename.

I looked through Daniel's PHP Security tutorial, but it was a little bit too advanced for me.

Is my site protected from SQL injections?

When errors are encountered, a new error_log file (permissions: 0400) pops up in my directory. Does my site report errors in a safe way?

I switched over to using sessions rather than cookies. Here is how I did it.

If login was successfull:

session_start();
$_SESSION['username']= $username;

Then, at the top of each page, I put something like this to check if user is logged in:

session_start();
if(isset($_SESSION['username'])){header("location: index.php");}

Is there any problems with this way of doing it?

So 3 questions:

1. Safe from sql injections?

2. Safe error reporting?

3. Safe use of sessions?

Thanks for your help.

Sign In

Create Account Site - Is this secure?

Recommended Posts

condoravenue

Link to comment

Share on other sites

tomfmason

Link to comment

Share on other sites

condoravenue

Link to comment

Share on other sites

awjudd

Link to comment

Share on other sites

ignace

Link to comment

Share on other sites

condoravenue

Link to comment

Share on other sites

Browse

Activity

Important Information