Page 1 of 1

Trying To Modify Coding

Posted: Wed Oct 28, 2009 5:39 pm
by CoolAsCarlito
I was told that these are issues I am having with how my coding looks. Any help as to fixing my problems with these.

1) Calling die() is not an appropriate way of handling errors.
2) You should use some sort of salting along with the password hashing.
3) Your usage of stripslashes() seems to suggest that you are running with magic quotes on; turn it off.
4) You should separate your presentational logic from your business logic (separations of concerns).
5) You are assuming that $_POST['username'] and $_POST['password'] are both set. If they are not you will get an E_NOTICE. You should check that an index exists in an array before you use it (unless you already know it does).
6) You don't need to MySQL escape $_POST['password'] seeing as you aren't using it in any query.
7) You are selecting the same user from the database twice, which is obviously redundant.
8) Assuming your variable upholds the invariant that usernames are unique, you needn't use a while loop when fetching the info because there will only every be at most one row returned. Seeing as you've also verified that a row was actually returned, you don't even need to check that mysql_fetch_array() returns the result. If it doesn't it would be a bug in PHP.

Code: Select all

<?php 
 
require "backstageconfig.php";
require "backstagefunctions.php";
 
ob_start();
//if the login form is submitted
if(isset($_POST['submit']))
{
    // makes sure they filled it in
    if(!$_POST['username'] || !$_POST['password'])
    {
        die('You did not fill in a required field.');
    }
   $username = mysql_real_escape_string($_POST['username']); 
   $pass = mysql_real_escape_string($_POST['password']); 
 
    $check = mysql_query("SELECT * FROM users WHERE username = '".$username."'")or die(mysql_error());
 
    //Gives error if user dosen't exist
    $check2 = mysql_num_rows($check);
    if ($check2 == 0)
    {
        die('That user does not exist in our database.');
    }
    while($info = mysql_fetch_array( $check )) 
    {
        $pass = md5(stripslashes($_POST['password']));
        $info['password'] = stripslashes($info['password']);
        //$_POST['pass'] = md5($_POST['pass']); THIS IS DONE IN THE ABOVE STATEMENT
        //gives error if the password is wrong
        if ($pass != $info['password'])
        {
            die('Incorrect password, please try again.');
        }
        else 
      
      // if login is ok then we add a cookie and send them to the correct page
        { 
            $username = stripslashes($username); 
         $_SESSION['username'] = $username; 
         $_SESSION['loggedin'] = time();
            
            // Finds out the user type
            $query = "SELECT `admin` FROM `users` WHERE `username` = '" . $username . "'";
            $result = mysql_query($query) or die(mysql_error()); 
            $row = mysql_fetch_array($result); 
            $admin = $row['admin'];
         $_SESSION['admin'] = $admin;
 
#########################################
######## ADMIN SCRIPT CAN BE ADDED BELOW
#########################################
if(isset($_SESSION['admin'])) { ?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta http-equiv="Content-Style-Type" content="text/css">
<meta http-equiv="Content-Language" content="en-us">
<meta name="language" content="en-us">
<title>Backstage V1 Administration Console</title>
<link rel="stylesheet" href="backstage.css" type="text/css" media="screen">
</head>
<body>
<div id=container>
<div class=header>
<table cellpadding="0" cellspacing="0" border="0" width="95%">
<tr>
<td width=110 align=center></td>
<td></td>
<td width=40 valign=bottom align=right>
<a href="#" onclick="">Home</a> | <a href="#" onclick="">Logout</a> | <a target="_blank" href="http://kansasoutlawwrestling.com/phpBB3">Forums</a></td>
</tr>
</table>
</div>
<div id=container2>
<div id=nav>
<?php if(isset($_SESSION['loggedin']))   { ?>
<h1>Character</h1>
<ul>
<li><a href="#" onclick="">Biography</a></li>
<li><a href="#" onclick="">Allies</a></li>
<li><a href="#" onclick="">Rivals</a></li>
<li><a href="#" onclick="">Quotes</a></li>
</ul>
<?php } ?>
<?php if(isset($_SESSION['loggedin']))   { ?>
<h1>Submit</h1>
<ul>
<li><a href="#" onclick="">Roleplay</a></li>
<li><a href="#" onclick="">News</a></li>
<li><a href="#" onclick="">Match</a></li>
<li><a href="#" onclick="">Seg</a></li>
</ul>
<?php } ?>
<?php if(isset($_SESSION['loggedin']) && $_SESSION['admin'] == 1) { ?>  
<h1>Handler</h1>
<ul>
<li><a href="#" onclick="">Directory</a></li>
</ul>
<?php } ?>
<?php if(isset($_SESSION['loggedin']) && $_SESSION['admin'] == 1) { ?>  
<h1>Booking</h1>
<ul>
<li><a href="#" onclick="">Champions</a></li>
<li><a href="#" onclick="">Booker</a></li>
<li><a href="#" onclick="">Compiler</a></li>
<li><a href="#" onclick="">Archives</a></li>
</ul>
<?php } ?>
<?php if(isset($_SESSION['loggedin']) && $_SESSION['admin'] == 1) { ?>  
<h1>Fed Admin</h1>
<ul>
<li><a href="#" onclick="">Handlers</a></li>
<li><a href="#" onclick="">Characters</a></li>
<li><a href="#" onclick="">Applications</a></li>
<li><a href="#" onclick="">Event Names</a></li>
<li><a href="#" onclick="">Title Names</a></li>
<li><a href="#" onclick="">Match Types</a></li>
<li><a href="#" onclick="">Divisions</a></li>
<li><a href="#" onclick="">Arenas</a></li>
 
</ul>
<?php } ?>
<?php if(isset($_SESSION['loggedin']) && $_SESSION['admin'] == 1) { ?>  
<h1>Site Admin</h1>
<ul>
<li><a href="#" onclick="">Templates</a></li>
<li><a href="#" onclick="">Content</a></li>
<li><a href="#" onclick="">Bio Configuration</a></li>
<li><a href="#" onclick="">News Categories</a></li>
<li><a href="#" onclick="">Menus</a></li>
</ul>
<?php } ?>
</div>
<div id=content>
 
</div>
<div id="footer">Backstage 1 &copy; 2009
</div>
</div>
</div>
</body>
</html>
<?php  
#########################################
######## ADMIN SCRIPT HAS TO END ABOVE
#########################################
    }
        } 
    } 
} 
else 
{
// if they have not submitted the form
?>
<!DOCTYPE HTML PUBLIC "-//W3C//DTD HTML 4.01 Transitional//EN" "http://www.w3.org/TR/html4/loose.dtd">
<html>
<head>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1">
<meta http-equiv="Content-Style-Type" content="text/css">
<meta http-equiv="Content-Language" content="en-us">
<meta name="language" content="en-us">
<title>Backstage V1 Administration Console</title>
<link rel="stylesheet" href="backstage.css" type="text/css" media="screen">
</head>
<body>
<div id=login>
<form method="POST" action="/mybackstage/backstage.php">
<h1>KOW Backstage</h1>
<p><label>Username:<br><input type="text" name="username" id="log" tabindex="1"></label></p>
<p><label>Password:<br><input type="password" name="password" id="pwd" tabindex="2"></label></p>
<p style="text-align: center;"><input type="submit" class="button" name="submit" id="submit" value="Login &raquo;" tabindex="4"></p>
</form>
</div>
</body>
</html>
<?php
}
?>