ccritique on Login Authentication for SQL Injection

[kilbey1]

I grabbed this code from the Macromedia site and altered it to prevent SQL Injections. Does this appear 'safe'?

Also, I get this error on logout and am not certain how to correct it. It makes me wonder if people are actually 'logging out' also.

Warning: Wrong parameter count for session_unregister() in /home/httpd/ wow-erp/employees/index.php on line 132.

<?php
# FileName="Connection_php_mysql.htm"
# Type="MYSQL"
# HTTP="true"
$hostname_wowcorp = "localhost";
$database_wowcorp = "dbname";
$username_wowcorp = "username";
$password_wowcorp = "password";
$wowcorp = mysql_pconnect($hostname_wowcorp, $username_wowcorp, $password_wowcorp) or trigger_error(mysql_error(),E_USER_ERROR); 

// *** Start the session
session_start();
// *** Validate request to log in to this site.
$FF_LoginAction = $HTTP_SERVER_VARS['PHP_SELF'];
if (isset($HTTP_SERVER_VARS['QUERY_STRING']) && $HTTP_SERVER_VARS['QUERY_STRING']!="") $FF_LoginAction .= "?".htmlentities($HTTP_SERVER_VARS['QUERY_STRING']);
if (isset($HTTP_POST_VARS['username'])) {
  $FF_valUsername=(get_magic_quotes_gpc()) ? $HTTP_POST_VARS['username'] : addslashes($HTTP_POST_VARS['username']);
  $FF_valPassword=(get_magic_quotes_gpc()) ? $HTTP_POST_VARS['password'] : addslashes($HTTP_POST_VARS['password']);
  $FF_fldUserAuthorization="";
  $FF_redirectLoginSuccess="index.php?goto=login";
  $FF_redirectLoginFailed="index.php";
  $FF_rsUser_Source="SELECT username, password";
  if ($FF_fldUserAuthorization != "") $FF_rsUser_Source .= "," . $FF_fldUserAuthorization;
  $FF_rsUser_Source .= " FROM users WHERE username='" . $FF_valUsername . "' AND password='" . $FF_valPassword . "'";
  mysql_select_db($database_wowcorp, $wowcorp);
  $FF_rsUser=mysql_query($FF_rsUser_Source, $wowcorp) or die(mysql_error());
  $row_FF_rsUser = mysql_fetch_assoc($FF_rsUser);
  if(mysql_num_rows($FF_rsUser) > 0) {
    // username and password match - this is a valid user
    $MM_Username=$FF_valUsername;
    session_register("MM_Username");
    if ($FF_fldUserAuthorization != "") {
      $MM_UserAuthorization=$row_FF_rsUser[$FF_fldUserAuthorization];
    } else {
      $MM_UserAuthorization="";
    }
    session_register("MM_UserAuthorization");
    if (isset($accessdenied) && true) {
      $FF_redirectLoginSuccess = $accessdenied;
    }
    mysql_free_result($FF_rsUser);
    session_register("FF_login_failed");
	$FF_login_failed = false;
    header ("Location: $FF_redirectLoginSuccess");
    exit;
  }
  mysql_free_result($FF_rsUser);
  session_register("FF_login_failed");
  $FF_login_failed = true;
  header ("Location: $FF_redirectLoginFailed");
  exit;
}

function logout_button() {
?>
    	   <table width="100%"  border="0" cellspacing="2" cellpadding="2">
             <tr>
               <td valign="top"><div align="left"><input type="submit" name="back" value="<< Back" onClick="javascript:history.back()"></a></div></td>
               <td><div align="right">
   	 <form method="post" action="/employees/index.php?goto=logout">
                 <input type="submit" name="logout" value="Logout">
		 </form>               

		 </div></td>
             </tr>
           </table>

<?
}

include('../display.php'); 

switch ($goto)
{
	case "login":
			head('Employee Login - Welcome','38','45');

		?>
		<p><span class="bodytext"><strong><font color="#0099CC" size="3">Welcome to the Employee section</font></strong></span></p> 
<p><span class="bodytext">This little corner in cyberspace has been created to provide WOW Employees useful information about the company. We have started by providing a set of downloadable Employee Forms and a Corporate Directory with the Names and Phone Numbers of the employees in the Corporate Office. We have also created a section on Technical Resources&amp; White Papers. Employees are encouraged to contribute Articles, White Papers, etc. The idea is to build an Intellectual Property Database that all employees can access. We plan to add more sections like Employee News and Announcements, Bulletin Board, etc.. We encourage everyone visit this section periodically and send us your comments and suggestions to <a href="mailto:[e-mail]">[e-mail]</a>.</span></p> 
			<?
		logout_button();				
	break;
	case "corpdir":
			include "welcome-corpdir.php";
			logout_button();
	break;
	case "emp":
			include "welcome-emp.php";
			logout_button();
	break;
	case "useful":
			include "welcome-useful.php";
			logout_button();
	break;
	case "logout";
		session_unregister();
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">");
	default:
	head('Employee Login','38','0');
?> 
   <form method="post" action="<?=$PHP_SELF?>"> 
      <table width="309" border="0" cellspacing="0" cellpadding="5" align="CENTER" bgcolor="#e2e2e2"> 
        <tr> 
          <td width="124" class="bodytext"><strong>Username :</strong></td> 
          <td width="356"><input name="username" type="text" id="username" size="20"></td> 
        </tr> 
        <tr> 
          <td class="bodytext"><strong>Password :</strong></td> 
          <td><input type="password" name="password" size="20"></td> 
        </tr> 
        <tr> 
          <td>&nbsp;</td> 
          <td><input type="submit" name="submit" value="Sign On"></td> 
        </tr> 
      </table> 
    </form> 
<?php
} // end of SWITCH statement

foot();
mysql_free_result($rstLogin);
?> 

[PurpleMonkey]

Try using session_destroy() instead, as this works fine for me.

[kilbey1]

Thanks PM. 🙂 I tried both and here's my second revision. The first is NOT secure (ie. you can log in to a page by typing in the URL if known). IF statements in each CASE will redirect you appropriately based on session status.

<?php
# FileName="Connection_php_mysql.htm"
# Type="MYSQL"
# HTTP="true"
$hostname_wowcorp = "localhost";
$database_wowcorp = "dbname";
$username_wowcorp = "someuser";
$password_wowcorp = "somepassword";
$wowcorp = mysql_pconnect($hostname_wowcorp, $username_wowcorp, $password_wowcorp) or trigger_error(mysql_error(),E_USER_ERROR); 

// *** Start the session
session_start();
// *** Validate request to log in to this site.
$FF_LoginAction = $HTTP_SERVER_VARS['PHP_SELF'];
if (isset($HTTP_SERVER_VARS['QUERY_STRING']) && $HTTP_SERVER_VARS['QUERY_STRING']!="") $FF_LoginAction .= "?".htmlentities($HTTP_SERVER_VARS['QUERY_STRING']);
if (isset($HTTP_POST_VARS['username'])) {
  $FF_valUsername=(get_magic_quotes_gpc()) ? $HTTP_POST_VARS['username'] : addslashes($HTTP_POST_VARS['username']);
  $FF_valPassword=(get_magic_quotes_gpc()) ? $HTTP_POST_VARS['password'] : addslashes($HTTP_POST_VARS['password']);
  $FF_fldUserAuthorization="";
  $FF_redirectLoginSuccess="index.php?goto=login";
  $FF_redirectLoginFailed="index.php";
  $FF_rsUser_Source="SELECT username, password";
  if ($FF_fldUserAuthorization != "") $FF_rsUser_Source .= "," . $FF_fldUserAuthorization;
  $FF_rsUser_Source .= " FROM users WHERE username='" . $FF_valUsername . "' AND password='" . $FF_valPassword . "'";
  mysql_select_db($database_wowcorp, $wowcorp);
  $FF_rsUser=mysql_query($FF_rsUser_Source, $wowcorp) or die(mysql_error());
  $row_FF_rsUser = mysql_fetch_assoc($FF_rsUser);
  if(mysql_num_rows($FF_rsUser) > 0) {
    // username and password match - this is a valid user
    $MM_Username=$FF_valUsername;
    session_register("MM_Username");
    if ($FF_fldUserAuthorization != "") {
      $MM_UserAuthorization=$row_FF_rsUser[$FF_fldUserAuthorization];
    } else {
      $MM_UserAuthorization="";
    }
    session_register("MM_UserAuthorization");
    if (isset($accessdenied) && true) {
      $FF_redirectLoginSuccess = $accessdenied;
    }
    mysql_free_result($FF_rsUser);
    session_register("FF_login_failed");
	$FF_login_failed = false;
    header ("Location: $FF_redirectLoginSuccess");
    exit;
  }
  mysql_free_result($FF_rsUser);
  session_register("FF_login_failed");
  $FF_login_failed = true;
  header ("Location: $FF_redirectLoginFailed");
  exit;
}

function logout_button() {
?>
    	   <table width="100%"  border="0" cellspacing="2" cellpadding="2">
             <tr>
               <td valign="top"><div align="left"><input type="submit" name="back" value="<< Back" onClick="javascript:history.back()"></a></div></td>
               <td><div align="right">
   	 <form method="post" action="/employees/index.php?goto=logout">
                 <input type="submit" name="logout" value="Logout">
		 </form>               

		 </div></td>
             </tr>
           </table>

<?
}

include('../display.php'); 

switch ($goto)
{
	case "login":
	if (! session_is_registered("MM_UserAuthorization") ) {
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">");
				echo "You must log in.<P>"; 		
	} else {
			head('Employee Login - Welcome','38','45');
			?>
			<p><span class="bodytext"><strong><font color="#0099CC" size="3">Welcome to the Employee section</font></strong></span></p> 
<p><span class="bodytext">This little corner in cyberspace has been created to provide WOW Employees useful information about the company. We have started by providing a set of downloadable Employee Forms and a Corporate Directory with the Names and Phone Numbers of the employees in the Corporate Office. We have also created a section on Technical Resources&amp; White Papers. Employees are encouraged to contribute Articles, White Papers, etc. The idea is to build an Intellectual Property Database that all employees can access. We plan to add more sections like Employee News and Announcements, Bulletin Board, etc.. We encourage everyone visit this section periodically and send us your comments and suggestions to <a href="mailto:[e-mail]">[e-mail]</a>.</span></p> 
			<?
		logout_button();
	}	
	break;
	case "corpdir":
	if (! session_is_registered("MM_UserAuthorization") ) {
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">"); 		
	} else {
			include "welcome-corpdir.php";
			logout_button();
	}		
	break;
	case "emp":
	if (! session_is_registered("MM_UserAuthorization") ) {
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">"); 		
	} else {
			include "welcome-emp.php";
			logout_button();
	}		
	break;
	case "useful":
	if (! session_is_registered("MM_UserAuthorization") ) {
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">"); 		
	} else {
			include "welcome-useful.php";
			logout_button();
	}		
	break;
	case "logout";
		session_destroy();
	    session_unregister("MM_UserAuthorization");
		echo("<meta http-equiv=\"Refresh\" content=\"0;url=$PHP_SELF\">");
	default:
	head('Employee Login','38','0');
?> 
   <form method="post" action="<?php echo $FF_LoginAction?>"> 
      <table width="309" border="0" cellspacing="0" cellpadding="5" align="CENTER" bgcolor="#e2e2e2"> 
        <tr> 
          <td width="124" class="bodytext"><strong>Username :</strong></td> 
          <td width="356"><input name="username" type="text" id="username" size="20"></td> 
        </tr> 
        <tr> 
          <td class="bodytext"><strong>Password :</strong></td> 
          <td><input type="password" name="password" size="20"></td> 
        </tr> 
        <tr> 
          <td>&nbsp;</td> 
          <td><input type="submit" name="submit" value="Sign On"></td> 
        </tr> 
      </table> 
    </form> 
<?php
} // end of SWITCH statement

foot();
mysql_free_result($rstLogin);
?>