0

This is my register form,after i add validation and my record can't insert into database.But my validation are working.If i exchange the code of validation and mysql,all can insert into database even if got error in my validation.

<?php
$fnameErr=$lnameErr=$passErr=$repassErr=$icErr=$emailErr=$add1Err=$add2Err=$postErr=$mobileErr="";
$fname=$lname=$pass=$repass=$ic=$email=$add1=$add2=$postcode=$mobile="";

if (isset($_POST['submitbtn']))
{
    $fname    = $_POST['bname'];
    $lname    = $_POST['lname'];
    $pass     = $_POST['bpass'];
    $repass   = $_POST['bconpass'];
    $ic       = $_POST['bic'];
    $email    = $_POST['bemail'];
    $add1     = $_POST['badd1'];
    $add2     = $_POST['badd2'];
    $postcode = $_POST['bpostcode'];
    $mobile   = $_POST['bmobile'];
    $country  = $_POST['bcountry'];
    $state    = $_POST['bstate'];
    $city     = $_POST['bcity'];
    $gen      = $_POST['bgender'];

        if($fname==""||$lname==""||$pass==""||$repass==""||$ic==""||$email==""||$add1==""||$add2==""||$country==""||$state==""||$postcode==""||$mobile==""||$city==""||$gen=="")                 
    {
    ?>
        <script type="text/javascript">
            alert("Please fill in all the required informations.");
        </script>
    <?php
    }


 if (empty($errors) === true)
    {

            //bemail
            if (filter_var($_POST['bemail'], FILTER_VALIDATE_EMAIL) === false) 
            {
                $emailErr = 'A valid email address is required';

            }
            else if (email_exists($_POST['bemail']) === true) 
            {
                $emailErr= 'Sorry, the email \'' . $_POST['bemail'] . '\' is already in use';

            }

            //fname xx
            if (!preg_match("/^[A-Z][a-zA-Z -]+$/i",$_POST['bname'])) 
            {
                $fnameErr= 'Your first name cannot contain with any symbol and number';

            }

            //lname xx
            if (!preg_match("/^[A-Z][a-zA-Z -]+$/i",$_POST['lname']) )
            {
                $lnameErr= 'Your last name cannot contain with any symbol and number';

            }
            //ic xx
            if(!preg_match("/^\d{6}-\d{2}-\d{4}$/i", $_POST['bic'])) 
            {
                $icErr= 'Your ic cannot contain any character / must insert "-"';

            }
            //mobile xx
            if (!preg_match("/^\d{3}-\d{7}$/i", $_POST['bmobile'])) 
            {
                $mobileErr= 'Phone must comply with this mask: 010-1111111 or 0111-1111111';

            }
            //password
            if (strlen($pass) < 6) 
            {
                $passErr = 'Your password must be at least 6 characters';

            }

            //re-password
            if ($_POST['bpass'] !== $_POST['bconpass']) 
            {
                $repassErr= 'Your password do not match';

            }

            //add1 xx
            if (!preg_match("/^[a-zA-Z0-9 _.,:\"\']+$/i", $_POST['badd1'])) 
            {
                $add1Err = 'Address 1 must be only letters, numbers or one of the following';

            }

            //add2 xx
            if (!preg_match("/^[a-zA-Z0-9 _.,:\"\']+$/i",$_POST['badd2'])) 
            {
                $add2Err= 'Address 2 must be only letters, numbers or one of the following';

            }

            //postcode xx
            if (!preg_match("/^\d{5}$/i", $_POST['bpostcode'])) 
            {
                $postErr = 'Postcode must be 5 digits';

            }   

    ?>
        <script type="text/javascript">
        alert("Register have some error,please complete your informations.");
        </script>
    <?php
    }
    else
    {

        $result = mysql_query("select * from member where Member_Email='$email'");                                                 
        if (mysql_num_rows($result)==0)
        {
            mysql_query("insert into member(Member_Name,Member_Lname,Member_Pass,Member_IC,Member_Email,Member_Street1,Member_Street2,Member_Country,Member_State,Member_Postcode,Member_HP,Member_City,Member_Gen) VALUES ('$fname','$lname','$pass','$ic','$email','$add1','$add2','$country','$state','$postcode','$mobile','$city','$gen')");
            header("Location:all_login.php");           

        ?>
            <script type="text/javascript">
            alert('Registered successfully!');
            </script>
        <?php
        }

        else
        {
?>
            <script type="text/javascript">
                alert("email address already exists!");                                                                           
            </script>
<?php
        }
    }   

}       
?>

I'm so sorry with my poor english.

  • Are you getting an error from the server? – Itay Sep 09 '13 at 19:10
  • 2
    [**Please, don't use `mysql_*` functions in new code**](http://bit.ly/phpmsql). They are no longer maintained [and are officially deprecated](http://j.mp/XqV7Lp). See the [**red box**](http://j.mp/Te9zIL)? Learn about [*prepared statements*](http://j.mp/T9hLWi) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://j.mp/QEx8IB) will help you decide which. – Kermit Sep 09 '13 at 19:10
  • You are doing some filtering on your user input, but nevertheless I think the allowed characters for some of the address elements are sufficient to allow a SQL injection through to the database. This script therefore will probably get hacked, unless you properly escape your user input. – halfer Sep 09 '13 at 19:14
  • When fill in incorrect record,will pop up validation and when fill in the correct records,no have any error,but but can't insert into database. – user233711 Sep 09 '13 at 19:14
  • That whole huge line, `$fnameErr=$lnameErr=$passErr=$repassErr=$icErr=$emailErr=$add1Err=$add2Err=$postErr=$mobileErr=""; $fname=$lname=$pass=$repass=$ic=$email=$add1=$add2=$postcode=$mobile=""; ` is totally unnecessary... – user229044 Sep 09 '13 at 19:20
  • @FreshPrinceOfSO - I dont see anywhere that PHP 5.5.0 or above is in question...**Where do you see that**? Naturally, if user233711 _is_ using PHP above 5.5.0 he or she could consider, one day, refactoring to PDO or mysqli, but that would not help him or her now... – davidkonrad Sep 09 '13 at 19:20
  • **By building SQL with outside variables, you are leaving yourself wide open to SQL injection attacks.** Also, any input data with single quotes in it, like a name of "O'Malley", will blow up your SQL query. Please learn about using parametrized queries, preferably with the PDO module, to protect your web app. http://bobby-tables.com/php has examples to get you started, and [this question](http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) has many examples in detail. – Andy Lester Sep 09 '13 at 19:36
  • @meagar if i no have put that code,the error message will not display. – user233711 Sep 10 '13 at 08:37
  • @davidkonrad how is preventing technical debt and preventing vulnerable code not helping OP now? – PeeHaa Sep 26 '13 at 21:31

2 Answers2

0

if (empty($errors) === true)

At no point before this in the code you posted do you define $errors. As per the PHP doc on that function, if the variable doesn't exist then empty() will return true. So unless you've left something out, your code will always enter the first part of the if statement and not the second.

I suspect that you want to change that line to if (empty($errors) === false) so that you only enter that part of your code if there is an error.

DiMono
  • 3,308
  • 2
  • 19
  • 40
0

You should be really carefull for usage of variable like $test should be '.$test.' if string. Also you should use mysqli

Jorge Y. C. Rodriguez
  • 3,394
  • 5
  • 38
  • 61
Ozan
  • 1,191
  • 2
  • 16
  • 31
  • Changing to string concatonation from string injection will not protect from SQL injection. http://bobby-tables.com/php – nickrak Sep 10 '13 at 05:53