Skip to main content
Code Review

Return to Answer

replaced http://security.stackexchange.com/ with https://security.stackexchange.com/
Source Link

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security this on security.

in profile.php

  1. try to use a better variable name than $get and $check. maybe something like $row and $query would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
  3. Since your not checking for special characters in the username upon registration you need to htmlencode it when you echo it. change all instances of echo $username; to echo(htmlentities($username));

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security.

in profile.php

  1. try to use a better variable name than $get and $check. maybe something like $row and $query would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
  3. Since your not checking for special characters in the username upon registration you need to htmlencode it when you echo it. change all instances of echo $username; to echo(htmlentities($username));

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security.

in profile.php

  1. try to use a better variable name than $get and $check. maybe something like $row and $query would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
  3. Since your not checking for special characters in the username upon registration you need to htmlencode it when you echo it. change all instances of echo $username; to echo(htmlentities($username));
added 208 characters in body
Source Link

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security.

in profile.php

  1. try to use a better variable name than $get and $check. maybe something like $row and $query would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
  3. Since your not checking for special characters in the username upon registration you need to htmlencode it when you echo it. change all instances of echo $username; to echo(htmlentities($username));

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security.

in profile.php

  1. try to use a better variable name than $get. maybe something like $row would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security.

in profile.php

  1. try to use a better variable name than $get and $check. maybe something like $row and $query would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
  3. Since your not checking for special characters in the username upon registration you need to htmlencode it when you echo it. change all instances of echo $username; to echo(htmlentities($username));
Post Undeleted by DiverseAndRemote.com
added 294 characters in body
Source Link

for your register script:for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login scriptfor your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security .

in profile.php

  1. try to use a better variable name than $get. maybe something like $row would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.

for your register script:

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.

for your register script

$ln = (!empty($_POST['lname'])) ? $_POST['lname'] : '';
$un = (!empty($_POST['username'])) ? $_POST['username'] : '';
$em = (!empty($_POST['email'])) ? $_POST['email'] : '';
$em2 = (!empty($_POST['email2'])) ? $_POST['email2'] : '';
$pswd = (!empty($_POST['password'])) ? $_POST['password'] : '';
$pswd2 = (!empty($_POST['password2'])) ? $_POST['password2'] : '';
$d = date("y-m-d");
  1. use more descriptive variable names. looking at $ln, $un, $em, $em2 and $d is a real pain to look at lower in the code. Try $lastname, $username, etc.
  2. instead of using (!empty($_POST['fname'])) make it the reverse so you dont have to do a negate. try using (empty($_POST['fname'])) ? '' : $_POST['fname']
  3. Instead of $reg = @$_POST['reg']; and if($fn && $ln && $un && $em && $em2 && $pswd && $pswd2) try: $doRegistration = isset($_POST['reg']) && !( empty($_POST['lname']) || empty($_POST['username']) || empty($_POST['email']) || empty($_POST['email2']) || empty($_POST['password']) || empty($_POST['password2'])). Doing this will avoid opening a database connection to check the username if your not actually going to create an account. Only open a database connection when you have all your data lined up and ready to go.
  4. use constants when ever possible. so for if(strlen($un)>25||strlen($fn)>25||strlen($ln)>25) change 25 to something like USERNAME_MAX_LENGTH, FIRSTNAME_MAX_LENGTH and LASTNAME_MAX_LENGTH by defining it like so: define('USERNAME_MAX_LENGTH', 25); etc. do the same for password length also.

for your login script

  1. There is no need to filter your password and username with preg_replace before you use it in your prepared statement. Just use the pdo::bindParam function and it will escape the input for you.
  2. It's not a good idea to store the password in your session. See this on security .

in profile.php

  1. try to use a better variable name than $get. maybe something like $row would be more suitable.
  2. And you already know this but stop using mysql_* functions and change that over to PDO with prepared statements with param binding.
added 294 characters in body
Source Link
Loading
added 277 characters in body
Source Link
Loading
Post Deleted by DiverseAndRemote.com
Source Link
Loading
lang-php

AltStyle によって変換されたページ (->オリジナル) /