chibineku Posted August 20, 2009 Share Posted August 20, 2009 Hiya...this is my new improved registration script. I haven't yet created the registrationfailure page, but that is trivial. Does it look safe? Could I do more to make it safe, within reason? I am already using client side regex to make sure that input is free from anything except 0-9a-zA-Z -_@ but should I also do that on the client side? Beyond safe, does it look like it will cut down on silly errors, like empty fields? All fields are required except city, because I can mail people stuff without that, and there's no point someone registering who won't be ordering anything, since there's no social interaction or anything. Here it is: <?phpsession_start();include_once("db_include.php5");doDB();//sanitize input$esc_f_name = mysqli_real_escape_string($_POST["f_name"]);$tr_f_name = trim($esc_f_name);$esc_l_name = mysqli_real_escape_string($_POST["l_name"]);$tr_l_name = trim($esc_l_name);$esc_address = mysqli_real_escape_string($_POST["address"]);$tr_address = trim($esc_address);$esc_town = mysqli_real_escape_string($_POST["town"]);$tr_town = trim($esc_town);$esc_city = mysqli_real_escape_string($_POST["city"]);$tr_city = trim($esc_city);$esc_postcode = mysqli_real_escape_string($_POST["postcode"]);$esc_email = mysqli_real_escape_string($_POST["email"]);$tr_email = trim($esc_email);$esc_confirmEmail = mysqli_real_escape_string($_POST["confirmEmail"]);$tr_confirmEmail = trim($esc_confirmEmail);$esc_username = mysqli_real_escape_string($_POST["username"]);$tr_username = trim($esc_username);$esc_confirmUsername = mysqli_real_escape_string($_POST["confirmUsername"]);$tr_confirmUsername = trim($esc_confirmUsername);$esc_password = mysqli_real_escape_string($_POST["password"]);$tr_password = trim($esc_password);$esc_f_name = mysqli_real_escape_string($_POST["confirmPassword"]);$tr_f_name = trim($esc_confirmPassword);if(!$_POST) { //come directly via address bar header("Location: contact.php5"); exit;} else if((empty($tr_f_name) || empty($tr_l_name) || empty($tr_address) || empty($tr_town) |||| empty($tr_password) || empty($tr_confirmPassword) || empty($tr_username) || empty($tr_confirmUsername) || empty($tr_email) || empty($tr_confirmEmail) || ($tr_password != $tr_confirmPassword) || ($tr_username != $tr_confirmUsername) ||($tr_email != $tr_confirmEmail)) { //required fields not set - send them back header("Location: ".$_SERVER["PHP_SELF"].""); exit;} else { //already registered? $check_sql = "SELECT id FROM masterAroma WHERE email = ".$tr_email.""; $check_res = mysqli_query($mysqli, $check_sql) or die(mysqli_error($mysqli)); if(mysqli_num_rows($check_res) > 0) { //duplicate entry header("Location: registerfail.php5"); exit;} else { //create query $register_sql = "INSERT INTO aromaMaster (f_name, l_name, email, username, password, date_registered) VALUES ( '".htmlspecialchars($tr_f_name)."', '".htmlspecialchars($tr_l_name)."', '".htmlspecialchars($tr_email)."', '".htmlspecialchars($tr_username)."', PASSWORD('".$tr_password."'), now())"; $register_res = mysqli_query($mysqli, $register_sql) or die(mysqli_error($mysqli)); $userid = mysqli_insert_id($mysqli); $_SESSION["userid"] = $userid; $address_sql = "INSERT INTO aromaAddress (userid, address, town, city, postcode) VALUES ( '".$userid."', '".htmlspecialchars($esc_address)."', '".htmlspecialchars($esc_town)."', '".htmlspecialchars($esc_city)."', '".htmlspecialchars($esc_postcode)."')"; $address_res = mysqli_query($mysqli, $address_sql) or die(mysqli_error($mysqli)); header("Location: registerredirect.php5"); } mysqli_close($mysqli); ?> I have not inserted the trimmed version of the address information, as spaces here are credible, but did trim them to make sure they aren't empty fields (by empty I mean " "). Link to comment Share on other sites More sharing options...
justsomeguy Posted August 20, 2009 Share Posted August 20, 2009 You probably don't need all of those variables, you can just do something like this:$fname = mysqli_real_escape_string(trim($_POST["f_name"]));Trimming doesn't remove all spaces, it's not going to remove spaces in an address for example.In my applications, I only escape something once it goes in the query. If there's a problem with validation or something I want to be able to print the unescaped value back out, so I only escape it once it goes in a query. I usually don't access $_GET or $_POST directly, I use this function to get something from either, trim it, and strip slashes if necessary: function form_var($var, $default = ''){ $retval = $default; if (isset($_POST[$var])) $retval = $_POST[$var]; elseif (isset($_GET[$var])) $retval = $_GET[$var]; if (is_array($retval)) { foreach ($retval as $k => $v) { $retval[$k] = trim($v); if (get_magic_quotes_gpc()) $retval[$k] = stripslashes($v); } } else { $retval = trim($retval); if (get_magic_quotes_gpc()) $retval = stripslashes($retval); } return $retval;} e.g.:$fname = form_var('f_name');$sql = "INSERT INTO users (fname) VALUES ('" . mysql_real_escape_string($fname) . "')"; Link to comment Share on other sites More sharing options...
chibineku Posted August 20, 2009 Author Share Posted August 20, 2009 Well, I've done it now, lol. I changed a few things already, and I get no errors, but it doesn't handle duplicate entries the way it's supposed to. I can't see why it doesn't break and redirect properly. I changed the lines that said this: if(mysqli_num_rows($check_res) > 0) {to >= 1 (when checking if there are already entries with entered email address/usernames) and it just carries on like nothing happened when I deliberately register more than once. Link to comment Share on other sites More sharing options...
justsomeguy Posted August 20, 2009 Share Posted August 20, 2009 The email address in this query needs quotes around it:$check_sql = "SELECT id FROM masterAroma WHERE email = ".$tr_email.""; Link to comment Share on other sites More sharing options...
chibineku Posted August 20, 2009 Author Share Posted August 20, 2009 Ah, I had made a mistake in the escaping and trimming.Now I've fixed that, I get an error about the @ sign in my email address. I guess mysqli_real_escape_string doesn't play well with such symbols? Link to comment Share on other sites More sharing options...
justsomeguy Posted August 20, 2009 Share Posted August 20, 2009 No, that should be fine, I've never gotten an error from the escape function. About all it does is add slashes before a few characters, there's not really anything to cause an error. Link to comment Share on other sites More sharing options...
chibineku Posted August 20, 2009 Author Share Posted August 20, 2009 bah, you were right all the time! the select query condition should have beenWHERE email='".$tr_email."'";Sorry for being an idiot, thanks for the constant help Link to comment Share on other sites More sharing options...
skaterdav85 Posted August 21, 2009 Share Posted August 21, 2009 If you want, you could also use the strip tags function strip_tags() on all your form data so a hacker doesn't do any cross-site scripting. Link to comment Share on other sites More sharing options...
Recommended Posts
Archived
This topic is now archived and is closed to further replies.