Is anything wrong with this code?

Question

I used this code to create a user registration page in my website. I firstly connected to my database and then did the below codes ----->

<form action="index.php" method="post">
  <p id="usr1">Name : </p><input id="input1" placeholder="Username" type="text" name="username" required>   </br>
  </br>
  <p id="usr2">Password : </p><input id="input2" placeholder="Password" type="text" name="pwd" required>  </br>
  <p id="usr3">Password : </p><input id="input3" placeholder="Re-Type your password" type="text" name="cpwd" required>  </br>
  </br>
  <input id="sub" name="subbox" type="submit">
</form>

<?php
    if (isset($_POST['submit_button'])) {
      $username= $_POST['username'];
      $password=$_POST['pwd'];
      $conpwd=$_POST['cpwd'];
    }
    if ($password == $conpwd) {
      $query = "SELECT * FROM  login WHERE name='$username' ";
      $query_run = mysqli_query($con,$query);
      if (mysqli_num_rows($query_run) > 1) {
        echo '<script type="text/javascript">alert("This Username Already exists. Please try another username!")</script>';
        // the above code will check if the username is already taken or not.
      }else {
        $query = "insert into login values('$username' , '$password')";
                                $query_run = mysqli_query($con,$query);
        if ($query_run) {
             echo '<script type="text/javascript">alert("Registration Successful!")</script><a href="../">Click Here To Continue</a>';
             $_SESSION['username'] = $username;
             $_SESSION['password'] = $password;
             header( "Location: homepage.php");
        }else {
             echo '<script type="text/javascript">alert("Server Error. Please try again after a few minutes!")</script>';
        }
      }
    }else {
       echo "Please check and re-type both passwords";
    }
?>

But it always return some errors.This is what I see when i try to run the code

Is all of your code in one file? Or is the HTML bit in a different file that the other? — Stephan Stanisic, Dec 21 '16 at 16:35
`registration page` Today, 2016, your tryout is bad practise, because you are saving the passowrd plain into database. Better recode it all and read about passwordhashing in php first. SQL Injections are possible too. — JustOnUnderMillions, Dec 21 '16 at 16:38
*"Is anything wrong with this code?"* - Yes there is and error reporting would have helped you here. — Funk Forty Niner, Dec 21 '16 at 16:40
**Never store plain text passwords!** Please use PHP's [built-in functions](http://jayblanchard.net/proper_password_hashing_with_PHP.html) to handle password security. If you're using a PHP version less than 5.5 you can use the `password_hash()` [compatibility pack](https://github.com/ircmaxell/password_compat). Make sure you ***[don't escape passwords](http://stackoverflow.com/q/36628418/1011527)*** or use any other cleansing mechanism on them before hashing. Doing so *changes* the password and causes unnecessary additional coding. — Jay Blanchard, Dec 21 '16 at 17:12
Thanks for helping me out. My problem was that i did not insert data to all of my columns in my database. Thank you guys for your help. — Evading Shadows, Dec 22 '16 at 00:32

score 2 · Answer 1 · edited May 23 '17 at 11:59

2

Is anything wrong with this code?

To answer your initial question, yes there is something wrong. Your code is vulnerable to SQL injection. You should have a look at: How can I prevent SQL injection in PHP? And password is stored plain in your database, which means no respect for your user. There are some other problems with code style but it's just bonus.

Anyway, the thing that cause you the "alert" problem is that submit_button button does not exists. There is no button with that name. Your if condition is always false. So you have to replace:

if (isset($_POST['submit_button'])) {

With

if (isset($_POST['subbox'])) {

And maybe add a value to your input (not sure it's required, I did not tested):

<input id="sub" name="subbox" type="submit" value="1">

Thanks to @Fred-ii-

edited May 23 '17 at 11:59

Community

1
1

answered Dec 21 '16 at 16:40

rap-2-h

23,287
23
130
217

even though it's vulnerable to an sql injection, doesn't solve the question. Look at their code again and very carefully. – Funk Forty Niner Dec 21 '16 at 16:42
@Fred-ii- You mean I missed something obvious that generates the "error"? Anyway the question is: "Is anything wrong with this code?" (in the title) so I know it's a bit troll, BUT I actually answered the question. – rap-2-h Dec 21 '16 at 16:47
Look at this `if (isset($_POST['submit_button'])) {...}` guess what happens here... ok I'll tell you: "nothing". – Funk Forty Niner Dec 21 '16 at 16:51
Haha! Ok thanks! You want to post your own answer or I edit mine? – rap-2-h Dec 21 '16 at 16:54
I would post an answer but fear a possible rabbit hole. You're more than welcome to edit your answer in accordance to it; you have my blessing ;-) – Funk Forty Niner Dec 21 '16 at 16:56
@Fred-ii- Thank you! – rap-2-h Dec 21 '16 at 17:03

Is anything wrong with this code?

1 Answers1