Sanitizing basic form input (Form Security)

I have a signup page for the restricted access section of my site, where users signup for a userid and their password is generated, then mailed to them. I need to sanitize these inputs to prevents javascript and SQL injection hacks (and whatever I can sanitize them against). I am trying to use the following code:

<?php // signup.php
include("errorhandler.php");
include("databaseconnect.php"); 
if (!isset($_POST['submitok'])):
    // Display the user signup form
    ?>
<!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> New User Registration </title>
  <meta http-equiv="Content-Type"
    content="text/html; charset=iso-8859-1
</head>
<body>

<h3>New User Registration Form</h3>
<p><font color="orangered" size="+1">
  <link href="stylesheet" type="text/css" />
<tt><b>*</b></tt></font>
   indicates a required field</p>
<form method="post" action="<?=$_SERVER['PHP_SELF']?>">
<table width="326" border="0" cellpadding="0" cellspacing="0">
    <tr>
        <td width="125" align="right">
            <p id="tableTrash">User ID: &nbsp;</p>
      </td>
        <td width="186" valign="middle">
            <input name="newid" type="text" maxlength="100" size="25" />
            <font color="orangered" size="+1"><tt><b>*</b></tt></font>
        </td>
    </tr>
    <tr>
        <td align="right">
            <p id="tableTrash">Full Name:&nbsp;</p>
      </td>
        <td valign="middle">
            <input name="newname" type="text" maxlength="100" size="25" />
            <font color="orangered" size="+1"><tt><b>*</b></tt></font>
        </td>
    </tr>
    <tr>
        <td align="right">
            <p  id="tableTrash">E-Mail Address:&nbsp;</p>
      </td>
        <td valign="middle">
            <input name="newemail" type="text" maxlength="100" size="25" />
            <font color="orangered" size="+1"><tt><b>*</b></tt></font>
        </td>
    </tr>
    <tr valign="top">
        <td height="25" align="right">
        	<p  id="tableTrash">User Type:&nbsp;</p>
      </td>
        <td valign="middle" width="160px">
           <select name="newusertype" id="usertype" title="User Type">
           <option value="a" selected="">Co-op Representative</option>
           <option value="b">Grower</option>
           <option value="c">Patient Collective</option>
           </select><font color="orangered" size="+1"><tt><b>*</b></tt></font>
        </td>
    </tr>
    <tr>
        <td height="41" colspan="2" align="right">
            <hr noshade="noshade" />
            <input type="reset" value="Reset Form" />
            <input type="submit" name="submitok" value="   OK   " />
        </td>
    </tr>
</table>
</form>
</body>
</html>
      <?php
else:
    // Process signup submission
    dbConnect('members1');
	
	$userid = ereg_replace("[^A-Za-z0-9]", "", $_POST['userid']); // filter everything but numbers and letters
	$newid = ereg_replace("[^A-Za-z0-9]", "", $_POST['newid']); // filter everything but numbers and letters
	$password = ereg_replace("[^A-Za-z0-9]", "", $_POST['password']); // filter everything but numbers and letters
	$newpass = ereg_replace("[^A-Za-z0-9]", "", $_POST['newpass']); // filter everything but numbers and letters
	$fullname = ereg_replace("[^A-Za-z0-9]", "", $_POST['fullname']); // filter everything but numbers and letters
	$newname = ereg_replace("[^A-Za-z0-9]", "", $_POST['newname']); // filter everything but numbers and letters
	$email = stripslashes($_POST['email']);
	$email = strip_tags($email);
	$email = mysql_real_escape_string($email);
	$newemail = stripslashes($_POST['newemail']);
	$newemail = strip_tags($newemail);
	$newemail = mysql_real_escape_string($newemail);
	$usertype = ereg_replace("[^a-z]", "", $_POST['usertype']); // filter everything but lowercase letters
	$newusertype = ereg_replace("[^a-z]", "", $_POST['newusertype']); // filter everything but lowercase letters	

    if ($_POST['newid']=='' or $_POST['newname']==''
      or $_POST['newemail']=='' or $_POST['newusertype']=='') {
        error('One or more required fields were left blank.\\\
'.
              'Please fill them in and try again.');
    }
    
    // Check for existing user with the new id
    $sql = "SELECT COUNT(*) FROM user WHERE userid = '$_POST[newid]'";
    $result = mysql_query($sql);
    if (!$result) {	
        error('A database error occurred in processing your '.
              'submission.\\\
If this error persists, please '.
              'contact webamaster@ccgc-ca.org.');
    }
    if (mysql_result($result,0,0)>0) {
        error('A user already exists with your chosen userid.\\\
'.
              'Please try another.');
    }
    
    $newpass = substr(md5(time()),0,6);
    
    $sql = "INSERT INTO user SET
              userid = '$_POST[newid]',
              password = '$newpass',
              fullname = '$_POST[newname]',
              email = '$_POST[newemail]',
              usertype = '$_POST[newusertype]'";
    if (!mysql_query($sql))
        error('A database error occurred in processing your '.
              'submission.\\\
If this error persists, please '.
              'contact webmaster@_\\\
' . mysql_error());
              
    // Email the new password to the person.
    $message = "Hello!

Your personal account for the CCGC Member Portal
has been created! To log in, proceed to the
following address:

    http://www.sitehome.members/

Your personal login ID and password are as
follows:

    userid: $_POST[newid]
    password: $newpass


If you have any problems, feel free to contact me at
<webmaster@_>.

-
 Your Site Webmaster
";

    mail($_POST['newemail'],"Your Password for the Project Website",
         $message, "From:Webmaster <webmaster@_>");
         
    ?>
    <!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> Registration Complete </title>
      <meta http-equiv="Content-Type"
        content="text/html; charset=iso-8859-1" />
    </head>
    <body>
    <p><strong>User registration successful!</strong></p>
    <p>Your userid and password have been emailed to
       <strong><?=$_POST['newemail']?></strong>, the email address
       you just provided in your registration form. To log in,
       click <a href="index.php">here</a> to return to the login
       page, and enter your new personal userid and password.</p>
    </body>
    </html>
    <?php
endif;
?>

But I tried to test it, and was abel to sign up for a userid of joeblow//\\?, for which my script gave me joeblow//\\\\?
Is this working? I though it was supposed to take out everything but numbers and letters? I guess if it’s going to add \\ to the end of any hack attempt, it probably wouldn’t work (for the hacker). Is what I have sufficiently secure?

Forget trying to filter out what you do not want, just permit in what you DO want.

For example: should a username be made up of the letters A-Z (a-z) numbers 0-9 a space and a dash?

If yes, then decide whether you abort if they cannot follow your onscreen instructions OR clean it up (dump everything outside of what you do allow) and carry on?


// rm all but Numbers, letters space dash 
$input = '0123 Big Street-South bc < < ?.,/#';
$output = preg_replace('#[^0-9a-z -]#i', '', $input);
echo $output . '<hr>';

// detect anything but Numbers, letters space dash between 6 and 12 chars long

if( preg_match("#^[0-9a-z -]{6,12}$#", $input) ) {
echo "all ok";
}else{
echo 'ABORT';
}

I don’t see how this is different from what I did. You used preg_replace, I used ereg_replace, what’s the difference? They both, as you said ‘filter out what I don’t want’, don’t they?

I am also still not sure if I need to filter out newid, or just userid?

I just read that ereg_replace has been deprecated in PHP 5.3. I am using, 5.2.17, but I guess it’s worth switching to the new syntax…I’ll let you know

Maybe it doesn’t work because I sanitized the variable at the wrong place in the code? Should I be doing this in the mysql query?

Currently, you strip out the characters you deem unwanted and proceed as normal. Sadly, when I go to login with anthony.sterling, the username I supplied, I will not be able to login despite receiving confirmation that I can.

This points back to what Cups has said, you need to add some logic to inform the user that the values they have submitted are disallowed.

It’s doing exactly what you told it to do…


[FONT=Courier New][COLOR=#007700]    [/COLOR][COLOR=#0000bb]$sql [/COLOR][COLOR=#007700]= [/COLOR][/FONT][FONT=Courier New][COLOR=#dd0000]"INSERT INTO user SET 
              userid = '$_POST[newid]', 
              password = '$newpass', 
              fullname = '$_POST[newname]', 
              email = '$_POST[newemail]', 
              usertype = '$_POST[newusertype]'"[/COLOR][COLOR=#007700]; [/COLOR][/FONT]

if you want to use the regex “cleaned” versions, you would want to use


[FONT=Courier New][COLOR=#007700]    [/COLOR][COLOR=#0000bb]$sql [/COLOR][COLOR=#007700]= [/COLOR][/FONT][FONT=Courier New][COLOR=#dd0000]"INSERT INTO user SET 
              userid = '$newid', 
              password = '$newpass', 
              fullname = '$newname', 
              email = '$newemail', 
              usertype = '$newusertype'"[/COLOR][COLOR=#007700]; [/COLOR][/FONT]

Oh, right! Thanks Dave!
I have re-defined that variable as “clean”, but haven’t used it anywhere.

But, now I see the problem Anthony is having. It will strip out his “.”, leaving him with “anthony sterling” as a user name, when he though he signed up as “anthony.sterling”. Two thoughts:
1.) we can tell users not to use symbols in their code (and even write code to let them if they entered illegal char’s. (too much work).
2.)We can rely on the confirmation email the let them know:
"Thank you for registering ‘anthony sterling’ your password is ‘xxxxx’. Maybe a note could be user to call their attention to the fact that illegal char’s might have been removed?

The thing is, the fact I’m using these “special characters” means my credentials are more secure, and sending someone their password in plain text is a big no no.

Just ask Sony. :wink:

Why not simply let you users use whatever characters they like for their username and password?

Now that you mention it, special chars do make it cryptographically more secure…

You mean, a simple mysql_real_escape_string() would be sufficient? I guess I am filtering to disallow javascript form being executed in my form. How would you protect against that?

By the way, I am still able to login with ‘joeblow%&{}??\/&’ as a user name. I have to assume that this isn’t working somehow. Why not?

What about PHPs built in santising and validating functions? E.g. filter_vat, filter_input etc. Surely that would be a good place to begin?

Seems to be so many different ways to do this, which one is the best? (and don’t say ‘it depends’, because I have posted my code)?

Sure, I’ll try ‘filter_var’ and ‘filter_input’. What the heck? I guess I am doing this the right way in the code, otherwise one of you gurus would have called me out? Just trying to get any of these filters going. How would you do it with my code?

OK, I tried these as well, and I get the same result. Is my assumption that this filter should not allow ?|';* in the user name correct? When I test it, I just try to sign up with a user name that has illegal char.'s in it.

Somehow none of my attempts to filter this data has produced any results.

What am I doing wrong?

I, and others, usually find it’s benificial to create a small test rig, much like this:


<?php
function isValidUsername($username){
  return !preg_match('~[^a-z0-9]~i', $username);
}

function isValidPassword($password){
  return !preg_match('~[^a-z0-9]~i', $password);
}

$tests = array(
  'anthony',
  'sterling',
  'anthony.sterling',
  'Anthony01',
  '@4-2345fss'
);

foreach($tests as $num => $test){
  echo 'Test: ', $num, PHP_EOL;
  printf(
    'Username: %s, Password: %s' . PHP_EOL,
    isValidUsername($test) ? 'PASS' : 'FAIL',
    isValidPassword($test) ? 'PASS' : 'FAIL'
  );
}

/*
  Test: 0
  Username: PASS, Password: PASS
  Test: 1
  Username: PASS, Password: PASS
  Test: 2
  Username: FAIL, Password: FAIL
  Test: 3
  Username: PASS, Password: PASS
  Test: 4
  Username: FAIL, Password: FAIL
*/

It will allow you to test the values, and your own expectations, within your code.