0

I am very new to php and I am trying to create a login system. Here is my code

<?php

    $con=mysqli_connect("XXXXXXX","XXXXXXXX","XXXXXXXXX","XXXXXXXXXX");

    if (!$con)
        {
        echo "failed to connect";
        }

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

    $sql = "SELECT userID FROM users WHERE username = $username and password =          $password;";

    if (!$sql) {
        echo 'query invalid'.mysql_error();
        }

    $result = mysql_query($sql);

    echo "$result";

    $row = mysqli_fetch_array($sql, MYSQLI_ASSOC);

    $active = $row['active'];

    $count = mysqli_num_rows($result);

    mysql_close($con);

?>

I am sure there is no problem with the connection. But my result is not showing up and there is no error message. Previously I had an IF statement that perform further action if the result comes back. Since I am trying to figure out what is going on, I just deleted that part. Somebody please help. Many thanks

Phiter
  • 14,570
  • 14
  • 50
  • 84
  • 1
    _here is my code_ - there is no code! – Jeff Oct 04 '16 at 00:59
  • 1
    He is new to Stack Overflow as well, guys. I editted it in. Be more comprehensive to newcomers. – Phiter Oct 04 '16 at 00:59
  • ah, here it is! You forgot the '' around username and password! – Jeff Oct 04 '16 at 01:00
  • sorry, this is my first question. – 王建捷 Oct 04 '16 at 01:01
  • @PhiterFernandes sorry, I couldn't even see a link or something similar – Jeff Oct 04 '16 at 01:01
  • This is very common when people copy paste their code but are unfamiliar with the way SO deals with code. They mostly don't know how to properly format it. So you just click edit and 99% chance the code will be there. – Phiter Oct 04 '16 at 01:04
  • 2
    You are open to SQL injections, passwords should be hashed. – chris85 Oct 04 '16 at 01:10
  • 1
    @Jeff... maybe `$username` already has the quotes included as part of the value, being passed that way from the form? How can we tell? We can see that the code is using a mix of client interface libraries (mysqli and mysql, which isn't going to work, and when that issue is corrected, the code appears to be vulnerable to SQL Injection. And if the passwords are being hashed, that operation is being performed in the wrong place, on the client rather than the server. The perfect trifecta: 1) invalid mix of mysqli and mysql functions 2) sql injection vulnerabilities, 3) plaintext passwords. – spencer7593 Oct 04 '16 at 01:20
  • with your current code your `echo 'query invalid'` line wont work. currently `$sql` is a string as such `if (!$sql)` wont ever work because you'll never have an empty string, false boolean, 0 as string or int or a null value. even if it did your mixing `mysql_*` with `mysqli_*` so you wouldn't even get any connection error and because your query hasn't even been executed you can forget about catching any invalid queries with it – Memor-X Oct 04 '16 at 01:23
  • [mysqli](http://stackoverflow.com/a/33665819/1816093) and [PDO](http://stackoverflow.com/a/32556010/1816093) of generally what you are trying to do. Includes session, hash, password_verify, exceptions, Prepared Statements. – Drew Oct 04 '16 at 01:29
  • If someone finds a better dupe target be my guest :p – Drew Oct 04 '16 at 01:32

2 Answers2

1

You're missing the single quotes around the variables, and also you're using mysql mixed with mysqli, which will not work.

$sql = "SELECT userID FROM users WHERE username = '$username' and password = '$password';";
$result = mysqli_query($con, $sql);

$row = mysqli_fetch_assoc($sql); // same as fetch_array with MYSQLI_ASSOC

$active = $row['active'];

$count = mysqli_num_rows($result);

You don't need mysql_close at the end, but if you want to use it, it's mysqli_close($con); Keep in mind this is unsafe.

Use this function to filter user input:

$username = mysqli_real_escape_string($con, $_POST['username']);
$password = mysqli_real_escape_string($con, $_POST['password']);

Read this to make sure your code follows the security standards.

Community
  • 1
  • 1
Phiter
  • 14,570
  • 14
  • 50
  • 84
0

I recommend this.

$sql = "SELECT userID FROM users 
WHERE username = " ._sql($username) ." and password = " ._sql($password);

// generate sql safe string function 

function _sql($txt) { 
    if ($txt === null || $txt === "") 
      return "NULL"; 
    if (substr($txt, 0, 2) == "##") 
      return substr($txt, 2); 
    //$txt = str_replace("'", "''", $txt); 
    $txt = mysql_real_escape_string($txt); 
    return "'" . $txt . "'"; 
}
Zhao.YM
  • 61
  • 1
  • 1
  • 11
  • 1
    What is this? Why do you recommend this? – chris85 Oct 04 '16 at 01:10
  • You need to structuring code before start – Zhao.YM Oct 04 '16 at 01:26
  • 1
    What is your answer proposing? What is `_sql()`? – chris85 Oct 04 '16 at 01:27
  • 1
    I am quite confuzled too – Drew Oct 04 '16 at 01:30
  • Personally, I'd recommend using a *prepared statement* with *bind placeholders*. I'd also recommend *not* storing plaintext passwords. If we're going to recommend using a function named `_sql`, it might be beneficial to provide some information about it... like the definition of the function, or if it's part of a package, a link to the documentation about how to install the package, etc. AFAIK `_sql` is not a function natively provided by PHP or the mysqli interface library. – spencer7593 Oct 04 '16 at 01:32
  • Zhao put that in via [edit] not down in comments. If I screwed it up please fix via [edit]. Welcome to the stack. hang in there :p – Drew Oct 04 '16 at 01:55
  • Sorry for my mistakes, I uploaded new code in a new form. – Zhao.YM Oct 04 '16 at 01:59