0
\$\begingroup\$

So I'm working on a register form and I have four fields: name, username, email and password. I pick up the values of these fields in jQuery and depending on if all the fields are filled, I pass them onto a PHP script via ajax. Is that safe for form validations? I was worried about data getting manipulated by the user.

Further on in the php script, I check if all the posted values have data in them, only then will I proceed onto doing some validations... The validations are the parts where I'm worried that it's not the best and there are many flaws with it.

$name = $_POST['name'];
$username = $_POST['username'];
$email = $_POST['email'];
$password = $_POST['password'];


if (!preg_match("/^[A-Za-z '-]+$/i", $name)) {

    $errors = "Please enter a valid name.";

} else if (!preg_match("/^[A-Za-z]+\s[A-Za-z]+$/", $name)) {

    $errors = "Please enter your first and last name.";

} else if (strlen($username) <= 3 || strlen($username) > 15) {

    $errors = "Please pick a username between 4 - 15 characters. Be creative.";

} else if (!preg_match("/^[A-Za-z][A-Za-z0-9_.-]{4,15}$/", $username)) {

    $errors = "Please pick an alphanumeric username between 4 - 15 characters.";

} else if (filter_var($email, FILTER_VALIDATE_EMAIL) === false) {

    $errors = "Please enter a valid email.";

} else if (strlen($username )< 6 || strlen($username) > 32) {

    $errors = "Your password must atleast be 6 characters.";

} else {

    echo "valid";

}

Are these validation steps secure? Are there any loop holes that the user can manipulate the data?

The requirements for the fields are as follows - which im not so sure i hit on the head with my code: - The username should have alphanumeric characters with underscores (optional) - the name should have BOTH the first name and the last name

Thank you.

\$\endgroup\$
  • \$\begingroup\$ What are you doing with the data? Putting it into a DB? into a file? ... \$\endgroup\$ – Pinoniq Aug 26 '13 at 7:19
2
\$\begingroup\$

I take it you're trying to protect yourself from XSS attacks, injection and general mallicious input. You're well on your way (but do have a look here, there are some interesting links at the bottom).

That said, I'd say your validation needs work on a couple of (crucial) points:

  • Don't be too strict: /^[A-Za-z '-]+$/i for a name doesn't allow "pièrre joão", but it clearly is a name. Just match all unicode chars, and drop the i flag, or change [A-Za-z] to [a-z]. I'd suggest you using this code: $name = trim($name); preg_match('/^[\w \'-]+$/u', $name). read about the u modifier here
  • Be consistent: You're applying 2 patterns to $name: /^[A-Za-z '-]+$/i and /^[A-Za-z]+\s[A-Za-z]+$/ This means that, the second time dashes and accents aren't allowed. That's not good, not good at all. If needs must, explode the $name on ' ' (a space), and apply the first pattern to each name individualy. Or work on the regex some more.
  • /^[A-Za-z][A-Za-z0-9_.-]{4,15}$/: This doesn't allow a 4 char long username, but requers the username to be at least 5 chars long: [a-z] matches 1 char, [a-z0-9_.-]{4,15} followed by at least 4, at most 15 chars. Now that's not bad, but it's not what you want.
  • Validate the right things: near the end, you do some checks on the password, but you're using the $username variable... typo?
  • Don't limit the length of a password: strlen($username) > 32? Why? If the client wants to use a longer (and possibly stronger) password, let them do so...
    If you're going to do some checks on the password, at least to make sure that they're using chars, numbers and symbols. Not just "somealllowercasepass", but "S0m3@llw@kkyp@55"

Like I said above, you're missing out on some things, too:

  • No where are you calling strip_tags to ensure any markup is being removed from input
  • The password validation isn't applied to the correct variable, and should be improved, too
  • Your error messages are waay too specific. That's a bad idea! If I had bad intentions, and tried to fill in the form, each time trying some XSS trickery, and you give me feedback as in: "username mustn't contain chars like <, >, /, \, #, %", I can move on and try to exploit the next input field, and make sure I fill in a valid username, until I find a field where you let your guard down...
    Keep your error messages general: "Not all input was correct/the expected format/met the requirements". That should be enough. But as DmitriZaitsev says: be careful not to alienate/offend the end-users, or your queste for safety might backfire (and leave you with no data to protect). A simple way of validating the email address and not restricting the username is to use the email address as username. If google and SO do it, why can't you?
\$\endgroup\$
  • \$\begingroup\$ Great advice except the last one where I would respectfully disagree from usability point of view. It always drives me mad when sites I barely interested in start wasting my time by rejecting my usernames, passwords etc for unclear reasons. First, there is no reason for them even to use 'username' instead of emails. Second, if they really want my attention, they better tell me quickly what is wrong with credentials I am inputting. An they better not be picky before I decide the site is not worth my time. Not every site needs the same security as bank. \$\endgroup\$ – Dmitri Zaitsev Sep 2 '13 at 9:29
  • \$\begingroup\$ Here is some more explanation on the topic: http://stackoverflow.com/questions/18356407/how-to-merge-user-data-after-login/18443311#18443311 \$\endgroup\$ – Dmitri Zaitsev Sep 2 '13 at 9:32
  • \$\begingroup\$ @DmitriZaitsev: I've said in my answer that you shouldn't restrict usernames/passwords too much (using the email is a good idea, BTW, I'll add that to my answer). The only think I was getting at with my last comment was that, if an invalid email address was submitted, don't say: an email must contain an @, but just add a notice, saying: not all fields were filled in correctly. have the UI set a red border around the textfield that contains bad input, if you must \$\endgroup\$ – Elias Van Ootegem Sep 2 '13 at 9:32
  • \$\begingroup\$ I don't see any problem telling the user that email entered is missing @. A hacker knows this already, but a user can be rightfully confused. Whenever your message is vague, you force the user think why, instead of helping. Think of a lazy user exhausted after work or crazy party or under hangover and trying to have some fun - and now he has to play guess games with the site. \$\endgroup\$ – Dmitri Zaitsev Sep 2 '13 at 9:56
  • 1
    \$\begingroup\$ @EliasVanOotegem BTW, upvotes for this and other of your insightful code reviews \$\endgroup\$ – Dmitri Zaitsev Sep 2 '13 at 10:21

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.