validation function skips captcha

[dsp77]

Hello,

I have created a sing up form with email, password and captcha. I verify at submit all the possible errors and it works except one, for example you fill the email field with non existing email in db, the 2 password field with same pass and fill the captcha field wrong. In this example it will proceed inserting the data in the db, i cannot figure what is wrong please advise.

register.php

<?php
session_start();

include('include/config.php');
include('include/validation.php');
include('include/header.php');
?>

<div id="register">
  <h1>Inregistrare</h1>
  <?php if(isset($_POST['send']) && (!validateEmail($_POST['email']) || !validatePasswords($_POST['pass1'], $_POST['pass2']) || !validateRegister())): ?>
  <div id="error">
    <ul>
      <?php if(!validateEmail($_POST['email'])): ?>
      <li><strong>E-mail invalid:</strong> Introduceti o adresa de email valida.</li>
      <?php endif ?>
      <?php if(!validatePasswords($_POST['pass1'], $_POST['pass2'])): ?>
      <li><strong>Parole invalide:</strong> Parolele nu se potrivesc sau sunt mai mici de 5 caractere!</li>
      <?php endif ?>
      <?php if(!validateCaptcha($_POST['code'])): ?>
      <li><strong>Cod invalid:</strong> Cod invalid.</li>
      <?php endif ?>
      <?php if(!validateRegister()): ?>
      <li><strong>Exista:</strong> exista.</li>
      <?php endif ?>
    </ul>
  </div>
  <form method="post" id="register" action="<?php echo $_SERVER['SCRIPT_NAME']; ?>">
    <div>
      <label for="email">E-mail</label>
      <input id="email" name="email" type="text" />
      <span id="emailInfo">Adresa Dvs. de email va fi folosita pentru login.</span> </div>
    <div>
      <label for="pass1">Parola</label>
      <input id="pass1" name="pass1" type="password" />
      <span id="pass1Info">Parola trebuie sa contina cel putin 5 caractere.</span> </div>
    <div>
      <label for="pass2">Confirma Parola</label>
      <input id="pass2" name="pass2" type="password" />
      <span id="pass2Info">Confirma parola</span> </div>
    <div><img src="include/captcha.php" /></div>
    <div>
      <label for="code">Introduceti codul de mai sus</label>
      <input id="code" type="text" name="code" />
      <span id="codeInfo">Sunteti om?</span> </div>
    <div>
      <input id="send" name="send" type="submit" value="Inregistrare" />
    </div>
  </form>
  <?php elseif(isset($_POST['send'])):?>
  <div id="error" class="valid">
    <ul>
      <li><strong>Felicitari!</strong> Datele de login au fost trimise la adresa <?php echo $_POST['email']; ?></li>
    </ul>
  </div>
  <?php
  $email=trim($_POST['email']);
  $password=md5($_POST['pass1']);
  $query="INSERT INTO users (email, password) VALUES ('$email', '$password')";
  $result_query=mysql_query($query) or die('EROARE MYSQL');
  ?>
  <?php endif ?>
  <?php if(!isset($_POST['send'])):?>
  <form method="post" id="register" action="<?php echo $_SERVER['SCRIPT_NAME']; ?>">
    <div>
      <label for="email">E-mail</label>
      <input id="email" name="email" type="text" />
      <span id="emailInfo">Adresa Dvs. de email va fi folosita pentru login.</span> </div>
    <div>
      <label for="pass1">Parola</label>
      <input id="pass1" name="pass1" type="password" />
      <span id="pass1Info">Parola trebuie sa contina cel putin 5 caractere.</span> </div>
    <div>
      <label for="pass2">Confirma Parola</label>
      <input id="pass2" name="pass2" type="password" />
      <span id="pass2Info">Confirma parola</span> </div>
    <div><img src="include/captcha.php" /></div>
    <div>
      <label for="code">Introduceti codul de mai sus</label>
      <input id="code" type="text" name="code" />
      <span id="codeInfo">Sunteti om?</span> </div>
    <div>
      <input id="send" name="send" type="submit" value="Inregistrare" />
    </div>
  </form>
  <?php endif ?>
</div>
<?php
include('include/footer.php');
?>

validation.php

<?php
    function validateName($name){
        //if it's NOT valid
        if(strlen($name) < 4)
            return false;
        //if it's valid
        else
            return true;
    }
    function validateEmail($email){
        return ereg("^[a-zA-Z0-9]+[a-zA-Z0-9_-]+@[a-zA-Z0-9]+[a-zA-Z0-9.-]+[a-zA-Z0-9]+.[a-z]{2,4}$", $email);
    }
    function validatePasswords($pass1, $pass2) {
        //if DOESN'T MATCH
        if(strpos($pass1, ' ') !== false)
            return false;
        //if are valid
        return $pass1 == $pass2 && strlen($pass1) > 4;
    }
    function validateMessage($message){
        //if it's NOT valid
        if(strlen($message) < 10)
            return false;
        //if it's valid
        else
            return true;
    }
    function validateCaptcha($code) {
        if($code == $_SESSION['rand_code'])
        //if it's valid
            return true;
        else
        //if it's NOT valid
            return false;
    }
    function validateRegister() {
        $query_check_login="SELECT email FROM users WHERE email='".$_POST['email']."' LIMIT 1";
        $result_check_login=mysql_query($query_check_login) or die('MYSQL ERROR');
        $row_check = mysql_num_rows($result_check_login);
        if($row_check == 1){
        //if it's NOT valid
            return false;
        } else {
        //if it's valid
            return true;
        }
    }
?>

 

[Adam]

Your logic's just a little mixed up. You validate the email address format, the passwords match, and then that the email address isn't taken here:

 

<?php if(isset($_POST['send']) && (!validateEmail($_POST['email']) || !validatePasswords($_POST['pass1'], $_POST['pass2']) || !validateRegister())): ?>

 

At which point only if there's a problem do you proceed to validate the captcha in the next series of validation calls:

 

      <?php if(!validateEmail($_POST['email'])): ?>
      <li><strong>E-mail invalid:</strong> Introduceti o adresa de email valida.</li>
      <?php endif ?>
      <?php if(!validatePasswords($_POST['pass1'], $_POST['pass2'])): ?>
      <li><strong>Parole invalide:</strong> Parolele nu se potrivesc sau sunt mai mici de 5 caractere!</li>
      <?php endif ?>
      <?php if(!validateCaptcha($_POST['code'])): ?>
      <li><strong>Cod invalid:</strong> Cod invalid.</li>
      <?php endif ?>
      <?php if(!validateRegister()): ?>
      <li><strong>Exista:</strong> exista.</li>
      <?php endif ?>

 

You could add the validateCaptcha() call to the first if condition, but personally I'd restructure the code to be more like:

 

if (isset($_POST['send']))
{
    // perform validation here, if there's
    // a problem store the $error message
}

// check if there was a validation error
if (!empty($error))
{
    // register user
}
else
{
    // display form
}

 

Seems like a much cleaner structure.

[rwwd]

    function validateCaptcha($code) {
        if($_SESSION['rand_code'] == $code)
        //if it's valid
            return true;
        else
        //if it's NOT valid
            return false;
    }

 

You have the comparison parts the wrong way around.

 

Also ereg functions are now deprecated, use preg_ instead, and lastly, $_POST is a super global array, therefore you DONT need to pass them into functions as they are available throughout the scope of your project.

 

@MrAdam, Completely agree with you on the last point there, at least your way there is some error handling, and it's better to structure like that.

 

Rw

[Adam]

You have the comparison parts the wrong way around.

I don't think he does?

 

 

$_POST is a super global array, therefore you DONT need to pass them into functions as they are available throughout the scope of your project.

Granted, but it's better to make the function reusable and pass in the argument.

[dsp77]

Thank you for your quick responses i found that i didn't added the  !validateCaptcha($_POST['code']) on if(isset($_POST['send'].

Ill take in consideration your suggestions and about MrAdam's logic i agree but i waned to try something else.

 

Thank you again and good luck