unsider Posted July 7, 2008 Share Posted July 7, 2008 <?php session_start(); include('config.php'); // others... $name=$_POST['name']; $email=$_POST['email']; $pass1=$_POST['pw1']; $pass2=$_POST['pw2']; $ip = $_SERVER['REMOTE_ADDR']; if(isset($_POST['Submit'])){ // username validation if(empty($name)){ echo 'Please enter a username'; exit(); } $name = stripslashes($name); if(strlen($name) < 5){ echo "Username below 5 characters"; exit(); } else if(strlen($name) > 15){ echo "Username above 15 characters"; exit(); } else if(!eregi("^([0-9a-z])+$", $name)){ echo 'Invalid username characters'; } $query = "SELECT * FROM user WHERE uname= '$name' "; $result= mysql_query($query); $num=mysql_num_rows($result); if ($num > 0) { echo "Username already exists, please choose another."; exit(); } // password validation if(empty($pass1)){ echo "Please fill in the password feilds"; exit(); } else if(empty($pass2)){ echo "Please fill in the password feilds"; exit(); } else if("$pass1" !== "$pass2" ){ echo "Your passwords do not match."; exit(); } $pass1 = stripslashes($pass1); $pass2 = stripslashes($pass2); if(strlen($pass1, $pass2) < 4) { echo"Password under 4 chars"; } else if(strlen($pass1, $pass2) > 15) { echo"Password greater than 15 chars"; } else if(!eregi("^([0-9a-z])+$", ($pass1 = trim($pass1)))){ echo " Password contains invalid chars"; } else if(!eregi("^([0-9a-z])+$", ($pass2 = trim($pass2)))){ echo " Password contains invalid chars"; } // email validation if(empty($email)){ echo 'please enter an email address.'; exit(); } else{ $regex = "^[_+a-z0-9-]+(\.[_+a-z0-9-]+)*" ."@[a-z0-9-]+(\.[a-z0-9-]{1,})*" ."\.([a-z]{2,}){1}$"; if(!eregi($regex,$email)){ echo "Email invalid"; } $email = stripslashes($email); } if($_SESSION['security_code'] == $_POST['security_code'] && !empty($_SESSION['security_code'])) { unset($_SESSION['security_code']); } else { echo 'Please try again, you have provided an invalid security code'; exit(); } $query="INSERT INTO user (uname, pw, email, date_joined, ip, level, isbanned) VALUES ('$name',password('$pw1'),'$email',NOW(),'$ip','$level','no')"; if(!@mysql_query ($query)) { echo mysql_error(); } } ?> I don't expect a really thorough inspection, but I would appreciate if someone could skim it for obvious security/validation flaws, or other ways in which I could make my code more effecient/effective. EDIT: if this is the wrong section I apologize, I think I've seen someone post a 'please check validate' thread in here before, so I assumed it was ok Thanks. Quote Link to comment Share on other sites More sharing options...
DarkWater Posted July 7, 2008 Share Posted July 7, 2008 1) Why do you set all the $_POST variables BEFORE checking if the form was even submitted? o-O 2) On this line: else if(!eregi("^([0-9a-z])+$", $name)){ You can just use ctype_alnum(). Look it up. 3) You might want a cleaner error system than just echoing something out and exiting. Quote Link to comment Share on other sites More sharing options...
unsider Posted July 7, 2008 Author Share Posted July 7, 2008 1) Why do you set all the $_POST variables BEFORE checking if the form was even submitted? o-O 2) On this line: else if(!eregi("^([0-9a-z])+$", $name)){ You can just use ctype_alnum(). Look it up. 3) You might want a cleaner error system than just echoing something out and exiting. Ya that's the next thing on my list, so please excuse the pathetic echos Quote Link to comment Share on other sites More sharing options...
DarkWater Posted July 7, 2008 Share Posted July 7, 2008 Oh yeah. You can't do this: if(strlen($pass1, $pass2) < 4) { Quote Link to comment Share on other sites More sharing options...
unsider Posted July 7, 2008 Author Share Posted July 7, 2008 Oh yeah. You can't do this: if(strlen($pass1, $pass2) < 4) { How should it be done? Quote Link to comment Share on other sites More sharing options...
DarkWater Posted July 7, 2008 Share Posted July 7, 2008 It should be erroring you for that, actually. O_O And you only need to check one ($pass1) because you've already checked if they're the same.... Quote Link to comment Share on other sites More sharing options...
unsider Posted July 7, 2008 Author Share Posted July 7, 2008 It should be erroring you for that, actually. O_O And you only need to check one ($pass1) because you've already checked if they're the same.... I guess that's true, and haha it's not catching that error. Quote Link to comment Share on other sites More sharing options...
DarkWater Posted July 7, 2008 Share Posted July 7, 2008 Put: error_reporting(E_ALL); ini_set('display_errors', 1); At the top of your script. I think it'll show you the error. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.