Using GLOBAL variables as safely as possible.

[ears2001]

Hi, I would appreciate some help with my code as I try and strive for efficient safe code but feel I'm not quite there! I have a page that allows me to edit, update and delete items of kit(clothing) in a database. The page posts the information to itself and if it is free of errors it will execute the desired database function. This is all controlled by a switch statement and the varible posted are collected by this bit of code below:

 

function register_globals($form_array)
{
   foreach($form_array as $name => $value)
   {
       $GLOBALS[$name] = $value;
   }
}

 

The whole pages code is:

 

<?php 
session_start();
header("Cache-control: private"); 

//$user_level = $_SESSION['user_level'];
if(($_SESSION['user_lev'] == 3) || ($_SESSION['user_lev'] == 4))
{
include '../database_info.php';
include '../error.php';
register_globals($_POST);//register globals from post
register_globals($_GET);//register_globals($_GET)

switch($Submit) {

case "Update":
$clubhome=addslashes($clubhome);
$clubhome=htmlspecialchars($clubhome);
$teamhome=addslashes($teamhome);
$teamhome=htmlspecialchars($teamhome);
$feeshome=addslashes($feeshome);
$feeshome=htmlspecialchars($feeshome);
echo (pagetop_logged_in_NoImage());
		if($clubid == 1 )
				{
				$update = mysql_query("UPDATE clubinfo SET  clubhome='$clubhome', teamhome='$teamhome', feeshome='$feeshome' WHERE clubid='$clubid' ");
    				$number_rows = mysql_affected_rows();
				}//end of else
			else
				{
				//need to add a row for information
				$insert2= mysql_query("INSERT INTO clubinfo (clubhome, teamhome, feeshome)
				VALUES ('$clubhome', '$teamhome', '$feeshome')");
				$number_rows = mysql_affected_rows();
				}//end if no results*/

if ($number_rows=='0')
{
?>
<p class="top">No details have been changed!</p>
<?php
}
else
    {
?>
<p class="top">Club Information successfully updated!</p>
<?php
    }
?>
<p>
<form  enctype="multipart/form-data" <?php echo "action=\"$_SERVER[php_SELF]\" "; ?> method="post">
<input name="Submit" type="submit" class="button" value="Return">
</form>
</p>
<?php
echo (pagebottom_logged_in_NoImage ());
break;

default: //enter team section, continue takes you into case : continue (line 13)

echo (pagetop_logged_in_NoImage());

$select = "SELECT * FROM clubinfo WHERE clubid ='1'";

$results = mysql_query("$select");

	while($row = mysql_fetch_array($results)){
	foreach( $row AS $key => $val ){
		$$key = stripslashes( $val );
		}//close foreach
		$clubhome = stripslashes($clubhome);
		$teamhome = stripslashes($teamhome);
		$feeshome = stripslashes($feeshome);
		}
?>
<h2>Manage Club Information</h2>
<form class="clubinfo" id="clubinfo" enctype="multipart/form-data" <?php echo "action=\"$_SERVER[php_SELF]\" "; ?> method="post">
<fieldset>

<table>
    <tr>
      <td><label for="clubinfo"><strong>Update Club Info</strong> - This detail is shown on the home page.</label></td>
</tr>
<tr>
      <td><textarea name="clubhome" cols="50" rows="10" id="clubhome"><?php echo $clubhome; ?>
  </textarea></td>
</tr>
<tr>
      <td><label for="teaminfo"><strong>Update Team Home Info</strong> - This detail is shown on the Team Home page.</label></td>
</tr>
<tr>
      <td><textarea name="teamhome" cols="50" rows="10" id="teamhome"><?php echo $teamhome; ?>
  </textarea></td>
</tr>
<tr>
      <td><label for="feesinfo"><strong>Update Club Fees Info</strong> - This detail is shown on the Fees page.</label></td>
</tr>
<tr>
      <td><textarea name="feeshome" cols="50" rows="10" id="feeshome"><?php echo $feeshome; ?>
  </textarea></td>
</tr>
<input name="clubid" type="hidden" value="<?php echo $clubid; ?>">
<tr>
      <td><input name="Submit" type="submit" class="button" value="Update"></td>
</tr>
</table>
</fieldset>
          
</form>
<?php
echo (pagebottom_logged_in_NoImage ());
break ;

// end of switch statment
}
}//end of if
else
{
header("Location: ../members/membershome.php");
exit;
}

?>

 

 

PLEASE CAN SOMEONE - Help with making this code more secure if it needs it.

 

 

THANKS.

[ratcateme]

i am not sure what you are trying to do but $_GET _POST _SESSION _SERVER .... are all superglobals as of php 4.1 so you dont need to make the global that is just pointless.

 

Scott.

[genericnumber1]

function register_globals($form_array)
{
   foreach($form_array as $name => $value)
   {
       $GLOBALS[$name] = $value;
   }
}

 

this is a very dangerous function to use, and I would advise against using it at all.

 

If you MUST use it, the only true way to easily secure it would be to make sure the $name value is one you want via a switch statement, and that defeats the purpose of register_globals anyway.

 

Instead of using the values from the form as variables (eg. $clubhome) replace their usage with the $_POST or $_GET superglobals (eg. $_POST['clubhome']), depending on whether your form is set as GET or POST.

 

I would advise changing your script to something like this... (untested)

<?php 
session_start();
header("Cache-control: private"); 

//$user_level = $_SESSION['user_level'];
if(($_SESSION['user_lev'] == 3) || ($_SESSION['user_lev'] == 4))
{
include '../database_info.php';
include '../error.php';

switch($Submit) {

case "Update":
$clubhome=addslashes($_POST['clubhome']);
$clubhome=htmlspecialchars($clubhome);
$teamhome=addslashes($_POST['teamhome']);
$teamhome=htmlspecialchars($teamhome);
$feeshome=addslashes($_POST['feeshome']);
$feeshome=htmlspecialchars($feeshome);
echo (pagetop_logged_in_NoImage());
		if($clubid == 1 )
				{
				$update = mysql_query("UPDATE clubinfo SET  clubhome='$clubhome', teamhome='$teamhome', feeshome='$feeshome' WHERE clubid='$clubid' ");
    				$number_rows = mysql_affected_rows();
				}//end of else
			else
				{
				//need to add a row for information
				$insert2= mysql_query("INSERT INTO clubinfo (clubhome, teamhome, feeshome)
				VALUES ('$clubhome', '$teamhome', '$feeshome')");
				$number_rows = mysql_affected_rows();
				}//end if no results*/

if ($number_rows=='0')
{
?>
<p class="top">No details have been changed!</p>
<?php
}
else
    {
?>
<p class="top">Club Information successfully updated!</p>
<?php
    }
?>
<p>
<form  enctype="multipart/form-data" <?php echo "action=\"$_SERVER[php_SELF]\" "; ?> method="post">
<input name="Submit" type="submit" class="button" value="Return">
</form>
</p>
<?php
echo (pagebottom_logged_in_NoImage ());
break;

default: //enter team section, continue takes you into case : continue (line 13)

echo (pagetop_logged_in_NoImage());

$select = "SELECT * FROM clubinfo WHERE clubid ='1'";

$results = mysql_query("$select");

	while($row = mysql_fetch_array($results)){
	foreach( $row AS $key => $val ){
		$$key = stripslashes( $val );
		}//close foreach
		$clubhome = stripslashes($clubhome);
		$teamhome = stripslashes($teamhome);
		$feeshome = stripslashes($feeshome);
		}
?>
<h2>Manage Club Information</h2>
<form class="clubinfo" id="clubinfo" enctype="multipart/form-data" <?php echo "action=\"$_SERVER[php_SELF]\" "; ?> method="post">
<fieldset>

<table>
    <tr>
      <td><label for="clubinfo"><strong>Update Club Info</strong> - This detail is shown on the home page.</label></td>
</tr>
<tr>
      <td><textarea name="clubhome" cols="50" rows="10" id="clubhome"><?php echo $clubhome; ?>
  </textarea></td>
</tr>
<tr>
      <td><label for="teaminfo"><strong>Update Team Home Info</strong> - This detail is shown on the Team Home page.</label></td>
</tr>
<tr>
      <td><textarea name="teamhome" cols="50" rows="10" id="teamhome"><?php echo $teamhome; ?>
  </textarea></td>
</tr>
<tr>
      <td><label for="feesinfo"><strong>Update Club Fees Info</strong> - This detail is shown on the Fees page.</label></td>
</tr>
<tr>
      <td><textarea name="feeshome" cols="50" rows="10" id="feeshome"><?php echo $feeshome; ?>
  </textarea></td>
</tr>
<input name="clubid" type="hidden" value="<?php echo $clubid; ?>">
<tr>
      <td><input name="Submit" type="submit" class="button" value="Update"></td>
</tr>
</table>
</fieldset>
          
</form>
<?php
echo (pagebottom_logged_in_NoImage ());
break ;

// end of switch statment
}
}//end of if
else
{
header("Location: ../members/membershome.php");
exit;
}

?>

 

I really only changed 3 lines, you may need to change more; I didn't look through the whole section of code.