Is this Form Secure?

Optimo · October 12, 2009

I have been doing some reading up on security of php forms and mysql insertion and such. As a result i have redesigned a form and below i have posted some of the code. Could some let me know if they think i have done all that is possible to ensure that the form is totally secure?

Are their any improvements possible and if so what are they?

<?php
include('session.php');

function mres($input) {
    return strip_tags(mysql_real_escape_string($input));
} 

if(isset($_POST['submit'])) 
{
if (isset($_SESSION['token']) && $_POST['token'] == $_SESSION['token'])
{
	$clean = array();
	$email_pattern = '/^[^@\s<&>]+@([-a-z0-9]+\.)+[a-z]{2,}$/i';

	$clean['username'] = ( isset( $_POST['username'] ) ) ? mres( $_POST['username'] ) : '';
	$clean['password'] = ( isset( $_POST['password'] ) ) ? mres(sha1( $_POST['password'] )) : '';
	$clean['password2'] = ( isset( $_POST['password2'] ) ) ? mres(sha1( $_POST['password2'] )) : '';
	if (preg_match($email_pattern, $_POST['email'])) 
	{ 
		$clean['email'] = mres($_POST['email']); 
	}
	if (preg_match($email_pattern, $_POST['email2'])) 
	{ 
		$clean['email2'] = mres($_POST['email2']); 
	}
	$clean['fname'] = ( isset( $_POST['fname'] ) ) ? mres( $_POST['fname'] ) : '';
	$clean['lname'] = ( isset( $_POST['lname'] ) ) ? mres( $_POST['lname'] ) : '';
	$clean['dobd'] = ( isset( $_POST['dobd'] ) AND ctype_digit( $_POST['dobd'] ) ) ? $_POST['dobd'] : 0;
	$clean['dobm'] = ( isset( $_POST['dobm'] ) ) ? mres( $_POST['dobm'] ) : '';
	$clean['doby'] = ( isset( $_POST['doby'] ) AND ctype_digit( $_POST['doby'] ) ) ? $_POST['doby'] : 0;
	$clean['sex'] = ( isset( $_POST['sex'] ) ) ? mres( $_POST['sex'] ) : '';
	$clean['country'] = ( isset( $_POST['country'] ) ) ? mres( $_POST['country'] ) : '';
	$clean['lordname'] = ( isset( $_POST['lordname'] ) ) ? mres( $_POST['lordname'] ) : '';
	$query = mysql_query("SELECT * FROM members WHERE username ='$clean[username]' ");
	$result = mysql_fetch_row($query);
	$query2 = mysql_query("SELECT * FROM resources WHERE lordname ='$clean[lordname]' ");
	$result2 = mysql_fetch_row($query2);

	if($clean['sex'] == "sexm")
	{
		$clean['sex'] = "male";
	}
	else
	{
		$clean['sex'] = "female";
	}

	if($result != 0)
	{
		$error = "Username taken please choose another";
	}
	else if(!($clean['password'] == $clean['password2']))
	{
		$error = "Your passwords do not match.";
	}
	else if(!($clean['email'] == $clean['email2']))
	{
		$error = "Your emails do not match.";
	}
	else if($result2 != 0)
	{
		$error = "Lordname taken please choose another";
	}
	else
	{		
		mysql_query("INSERT INTO mem 
				(id, username, user_password, fname, 
lname, email, dob, sex) 
		VALUES	('NULL','$clean[username]','$clean[password]','$clean[fname]',
'$clean[lname]','$clean[email]','$clean[dobd]-$clean[dobm]-$clean[doby]',
'$clean[sex]')
		") 
		or die(mysql_error()); 

		mysql_query("INSERT INTO res (lordname) VALUES	('$clean[lordname]')
		")		
		or die(mysql_error());

		echo"<p>Member Added";
	}
}
}

if(isset($error))
{
echo"$error";
}

$token = md5(uniqid(rand(), true));
$_SESSION['token'] = $token;
?>

<!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Transitional//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-transitional.dtd">
<html xmlns="http://www.w3.org/1999/xhtml">
<head>
<title>Member Signup</title>
<meta http-equiv="Content-Type" content="text/html; charset=iso-8859-1" />
</head>
<body>
<form name="login-form" id="login-form" method="post" action="signup.php">
<fieldset>
	<legend>Member Signup</legend>
	<dl>
		<dt>
			<label>Username: <input name="username" type="text" maxlength="20" id="username" /></label>
		</dt>
	</dl>
	<dl>
		<dt>
			<label>Password: <input name="password" type="password" maxlength="20" id="password" /></label>
			<label>Repeat Password: <input name="password2" type="password" maxlength="20" id="password2" /></label>
		</dt>
	</dl>
	<dl>
		<dt>
			<label>E-Mail: <input name="email" type="text" maxlength="100" id="email" /></label> 
			<label>Repeat E-Mail: <input name="email2" type="text" maxlength="100" id="email2" /></label>
		</dt>
	</dl>
	<dl>
		<dt>
			<label>First Name: <input name="fname" type="text" maxlength="100" id="fname" /></label>
			<label>Last Name: <input name="lname" type="text" maxlength="100" id="lname" /></label>
		</dt>
	</dl>
	<dl>
		<dt>
			<label title="Username">Date of Birth: 
			<label for="day">Day</label>

			<select name="dobd" id="day">
				<option value="1" selected="selected">1</option>
				...
			</select>

			<label for="month">Month</label>
			<select name="dobm" id="month">
				<option value="January" selected="selected">January</option>
				....
			</select>

			<label for="year">Year</label>
			<select name="doby" id="year">
			<option value="2008" selected="selected">2008</option>
				....
			</select>
		</label>
		</dt>
	</dl>
	<dl>
		<dt>
			<label>Sex: Male<input name="sex" type="radio" id="sex" value="sexm" />Female<input name="sex" type="radio" id="sex" value="sexf" /></label>
		</dt>
	</dl>
	<dl>
		<dt>  
			<label for="country">Country</label>
				<select name="country" id="country">
				<option value="NONE" selected="selected">Please Choose One</option>
				.....
			</select> 
		</dt>
	</dl>
	<dl>
		<dt>
			<label title="Username">Lord Name: 
			<input name="lordname" type="text" maxlength="25" id="lordname" />
			</label>
		</dt>
	</dl>
	<dl>
		<dt>
			<input type="hidden" name="token" value="<?php echo $token; ?>" />
			<label title="Submit"><input type="submit" name="submit" value="Signup" /></label>
		</dt>
	</dl>
</fieldset>
</form>
</body>
</html>

Thanks in Advance

Mike

mikesta707 · October 12, 2009

it seems fine. as far as mysql goes, usually you are good with mysql_real_escape_string(). you may want to add a trim for posterity, and I usually do an htmlentities rather than a strip tag.

Sign In

Is this Form Secure?

Recommended Posts

Optimo

Link to comment

Share on other sites

mikesta707

Link to comment

Share on other sites

Join the conversation

Browse

Activity

Important Information