pahunrepublic Posted September 14, 2010 Share Posted September 14, 2010 I made a registration form I wonder if it's secure enough: <?php include_once 'dbinfo.php'; if(isset($_POST['kuldes'])){ $name = trim($_POST['nev']); $username = $_POST['felh_nev']; $password = $_POST['password']; $email = $_POST['email']; $phone = $_POST['telefon']; $gender = $_POST['sex']; $hobby = $_POST['hobby']; $regfelt = $_POST['regfelt']; $name = strip_tags($name); $name = stripslashes($name); $username = strip_tags($username); $email = strip_tags($email); $phone = strip_tags($phone); $date = date("d-m-Y"); $errors = array(); if($name == NULL || $username == NULL || $password == NULL || $email == NULL || $phone == NULL){ $errors[] = "Please complete the form or one of the boxes is empty."; } else { // all the required form fields are present, validate data if(strlen($username) <= 3 || strlen($username) >= 30){ $errors[] = "Your username must be between 3 and 30 characters.."; } else { // username is valid length, check if already in use $stmt = $connect->prepare('SELECT username FROM users WHERE username=?'); $stmt->bind_param('s',$username); $stmt->execute(); $stmt->bind_result($username); while ($stmt->fetch()){ printf("Name: %s\n", $username); $errors[] = "The username is already in use!"; } $stmt->close(); } if(strlen($password) <= 6 || strlen($password) >= 12){ $errors[] = "Your password must be between 6 and 12 digits and characters.."; } if(!eregi("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})$", $email)){ $errors[] ="Your email address was not valid.."; } if(!eregi("^[0-9]{1,3}-[0-9]{1,3}-[0-9]{1,10}$",$phone)){ $errors[] ="Phone number is invalid. Only numbers with hyphen. Allowed format: countrycode-areacode-phonenumber"; } if(!isset($hobby)){ $errors[] ="Youd didn't select any hobbies"; } if(!isset($regfelt)){ $errors[] ="You didn't accept the terms"; } if(!isset($gender)){ $errors[] ="You didn't choose sex!"; } // all validation is done, process data if no errors if(empty($errors)){ $password = md5($password); $hobby = implode(",", $hobby); $stmt = $connect->prepare('INSERT INTO users(name,sex,email,phone_number,username,password,hobby) VALUES (?,?,?,?,?,?,?)'); $stmt->bind_param('sssssss', $name, $gender, $email, $phone, $username, $password, $hobby); $stmt->execute(); $stmt->close(); header("Location: login_form.php"); exit; } } } // prepare error string for form ?> <h1>Registration Form</h1> <form action="<?php echo $_SERVER['../MYSQLI/PHP_SELF']; ?>" name="registration_form" method="POST"> <p>Name: <input type="text" name="nev" value="<?php echo (isset($name) ? $name : ''); //ha $name változó meg lett adva akkor írja ki amit beírt ha nem akkor ''?>" size=25></p> <p>Username: <input type="text" name="felh_nev" value="<?php echo (isset($username) ? $username : ''); ?>" size=10></p> <p>Password: <input type="password" name="password" size=10></p> <!--<p>Password again:<input type="password" name="password_confirmation"></p>--> <p>E-mail: <input type="text" name="email" value="<?php echo (isset($email) ? $email : ''); ?>"/></p> <p>Phone number: <input type="text" name="telefon" value="<?php echo (isset($phone) ? $phone : ''); ?>"/></p> <p>Sex: <label><input type="radio" name="sex" value="no" >Female</label> <label><input type="radio" name="sex" value="ferfi" >Male</label></p> <p>Favorite hobbies (Using CTRL you can select more than one):</p> <select name="hobby[]" size="4" multiple> <option value="sport">Sport</option> <option value="mozi">Movies</option> <option value="kirandulas">Hiking</option> <option value="olvasas">Reading</option> </select> <!-- <p>Other message:</p> <textarea name="megjegyzes" cols="40"></textarea>--> <p><input name="regfelt" type="checkbox" value="elfogad">I accept the terms!</p> <p><input name="kuldes" type="submit" value="Submit form"> <input name="reset" type="reset" value="delete"></p> <table width="501" border="1"> <tr> <td><?php for($i=0; $i<count($errors); $i++){echo $errors[$i]."</br>";}?></td> </tr> </table> <p> </p> </form> Any ideas are welcome to make it bulletproof. Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/ Share on other sites More sharing options...
trq Posted September 14, 2010 Share Posted September 14, 2010 Secure enough for what? Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111015 Share on other sites More sharing options...
Pikachu2000 Posted September 14, 2010 Share Posted September 14, 2010 For starters, what is this? <form action="<?php echo $_SERVER['../MYSQLI/PHP_SELF']; ?>" name="registration_form" method="POST"> If you're trying to use $_SERVER['PHP_SELF'], don't. It's an XSS vulnerability. If you want to submit a form to itself, simply make it <form action="" . . ., or explicitly name the script in the action attribute. Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111025 Share on other sites More sharing options...
rwwd Posted September 14, 2010 Share Posted September 14, 2010 Hi there, not so much secure, portability wise, your using deprecated 'ereg' functions (>php5.3.0) you need to use preg_ instead... also:- if(isset($_POST['kuldes'])){ Would be better as :- if(isset($_POST['kuldes']) && !empty($_POST['kuldes'])){ This just makes sense, because with your version your checking that the key is set, but not checking to see if there is a value in there, this could be harmful if someone wanted to circumvent your form.. And it's always a good idea to have error_reporting(E_ALL); on at the top of the script that you are working on.. Cheers, Rw Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111032 Share on other sites More sharing options...
KevinM1 Posted September 14, 2010 Share Posted September 14, 2010 For starters, what is this? <form action="<?php echo $_SERVER['../MYSQLI/PHP_SELF']; ?>" name="registration_form" method="POST"> If you're trying to use $_SERVER['PHP_SELF'], don't. It's an XSS vulnerability. If you want to submit a form to itself, simply make it <form action="" . . ., or explicitly name the script in the action attribute. I believe one of $_SERVER['SCRIPT_NAME'] or $_SERVER['SCRIPT_FILENAME'] does that. Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111051 Share on other sites More sharing options...
pahunrepublic Posted September 16, 2010 Author Share Posted September 16, 2010 For starters, what is this? <form action="<?php echo $_SERVER['../MYSQLI/PHP_SELF']; ?>" name="registration_form" method="POST"> If you're trying to use $_SERVER['PHP_SELF'], don't. It's an XSS vulnerability. If you want to submit a form to itself, simply make it <form action="" . . ., or explicitly name the script in the action attribute. I'll just do all the suggestions you're giving me guys. I did the first one written by Nightsylr: <h1>Registration Form</h1> <form action="" name="registration_form" method="POST"> Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111653 Share on other sites More sharing options...
pahunrepublic Posted September 16, 2010 Author Share Posted September 16, 2010 Secure enough for what? If my code is secure enough for all the PHP vulnerabilities. Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111654 Share on other sites More sharing options...
pahunrepublic Posted September 16, 2010 Author Share Posted September 16, 2010 Hi there, not so much secure, portability wise, your using deprecated 'ereg' functions (>php5.3.0) you need to use preg_ instead... also:- if(isset($_POST['kuldes'])){ Would be better as :- if(isset($_POST['kuldes']) && !empty($_POST['kuldes'])){ This just makes sense, because with your version your checking that the key is set, but not checking to see if there is a value in there, this could be harmful if someone wanted to circumvent your form.. And it's always a good idea to have error_reporting(E_ALL); on at the top of the script that you are working on.. Cheers, Rw I did what you told me: if(!preg_match("^[_a-z0-9-]+(\.[_a-z0-9-]+)*@[a-z0-9-]+(\.[a-z0-9-]+)*(\.[a-z]{2,3})^", $email)){ $errors[] ="Your email address was not valid.."; } if(!preg_match("^[0-9]{1,3}-[0-9]{1,3}-[0-9]{1,10}^",$phone)){ $errors[] ="Phone number is invalid. Only numbers with hyphen. Allowed format: countrycode-areacode-phonenumber"; } Quote Link to comment https://forums.phpfreaks.com/topic/213398-is-this-script-secure-enough/#findComment-1111662 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.