-3

My login page will allow any credentials to login and I cannot figure out for the life of me why.

Code:

<?php require 'db_conn.php'?>
<?php
session_start();
$conn = sqlsrv_connect($serverName, $connectionInfo);
if ($conn === false) {
    die ('Failed to connect to Database');
}
if ( !isset($_POST['inputUsername'], $_POST['inputPassword']) ) {
    die ('Please fill both the username and password field!');
}
$usersql = "SELECT User, Password FROM Users WHERE User = '" . $_POST['inputUsername'] . "' AND Password = '" . $_POST['inputPassword'] . "'";
$userstmt = sqlsrv_query($conn, $usersql);
if ($userstmt) {
   $userrow = sqlsrv_fetch_array($userstmt, SQLSRV_FETCH_ASSOC);
   session_regenerate_id();
   $_SESSION['loggedin'] = TRUE;
   $_SESSION['name'] = $_POST['username'];
   $_SESSION['id'] = $id;
   header('Location: home.php');
}
else {
    $_SESSION['loggedin'] = FALSE;
    header('Location: login.php');
}

sqlsrv_close($conn);
?>

All help is appreciated.

Zhorov
  • 28,486
  • 6
  • 27
  • 52
Jackawan
  • 89
  • 10
  • 2
    Instead of only checking if the query succeeded (`if ($userstmt)`) you also need to check if there was any results, `$userrow`. Or by using `sqlsrv_num_rows()`. – Qirel Jul 21 '20 at 07:19
  • Please check the result of query of $userstmt. Please print the value of $userstmt – Mahesh Bhatnagar Jul 21 '20 at 07:21
  • 2
    You should also not store passwords in **plain text**, that is highly inapropriate! Would you like for others to ever see your password? No? Didn't think so either ;-) – Qirel Jul 21 '20 at 07:21
  • Amd in addition you should use prepared statements to avoid SQL injection attacks. – Qirel Jul 21 '20 at 07:21
  • Yeah I know about passwords being in plain text and SQL Injections, this is purely test data to get my head around it before I use live data. – Jackawan Jul 21 '20 at 07:25
  • A couple of links to back up what has already been said [How to use PHP's password_hash to hash and verify passwords](https://stackoverflow.com/questions/30279321/how-to-use-phps-password-hash-to-hash-and-verify-passwords) and [How can I prevent SQL injection in PHP?](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) – Nigel Ren Jul 21 '20 at 07:26
  • @Jackawan Be careful with the `user` column, because `USER` is a reserved keyword and `WHERE user = 'some_value'` can be very tricky. – Zhorov Jul 21 '20 at 12:18

3 Answers3

1

The actual reason for this unexpected behaviour is that you are checking the result from the sqlsrv_query() call, not the data returned from the executed statement.

You also need to consider the following:

  • You are using user in your SELECT statement and in your WHERE clause. USER is a reserved T-SQL keyword which returns the database user name, so you need to use [user] instead.
  • Use parameters in your statement to prevent possible SQL injection issues. As is mentioned in the documentation, the sqlsrv_query() function is well-suited for one-time queries and should be the default choice to execute queries unless special circumstances apply. This function provides a streamlined method to execute a query with a minimum amount of code. The sqlsrv_query function does both statement preparation and statement execution, and can be used to execute parameterized queries.
  • Use sqlsrv_has_rows() to check if the result set has one or more rows.
  • As an additional note, do not store passwords in plain text, use hashed passwords.

The next example (based on the code in the question) is a possible solution to your problem:

<?php require 'db_conn.php'?>
<?php
// Session
session_start();

// Connection
$conn = sqlsrv_connect($serverName, $connectionInfo);
if ($conn === false) {
    die ('Failed to connect to Database');
}

// Statement
if (!isset($_POST['inputUsername'], $_POST['inputPassword']) ) {
    die ('Please fill both the username and password field!');
}
$usersql = "SELECT [User], [Password] FROM Users WHERE [User] = ? AND [Password] = ?";
$userparams = array($_POST['inputUsername'], $_POST['inputPassword']);
$userstmt = sqlsrv_query($conn, $usersql, $userparams);

// 
if ($userstmt === false) {
    $_SESSION['loggedin'] = FALSE;
    header('Location: login.php');
    exit;
}   
if (!sqlsrv_has_rows($userstmt)) {
    $_SESSION['loggedin'] = FALSE;
    header('Location: login.php');
    exit;
}

// 
$userrow = sqlsrv_fetch_array($userstmt, SQLSRV_FETCH_ASSOC);
session_regenerate_id();
$_SESSION['loggedin'] = TRUE;
$_SESSION['name'] = $_POST['username'];
$_SESSION['id'] = $id;

sqlsrv_free_stmt($userstmt);
sqlsrv_close($conn);
header('Location: home.php');
?>
Zhorov
  • 28,486
  • 6
  • 27
  • 52
  • Now it doesn't let any login, it just keeps redirecting back to log in page. – Jackawan Jul 21 '20 at 07:43
  • @Jackawan We need a debug information - 1) What does `var_dump($userstmt)` return; 2) What are the data types of `User` and `Password` columns; 3) What are the actual values of `$_POST['username']` and `$_POST['inputPassword']` array items; 4) Possible unicode issues, if you miss `CharacterSet"=>"UTF-8"` option in the `sqlsrv_connect()` call. – Zhorov Jul 21 '20 at 07:48
0

sqlsrv_query() function returns statement resource which is a mixed adat type. It will false if there is an error in the query.

if ($userstmt) returns always true.

Senthilnadhan Ramasamy
  • 903
  • 7
  • 11
-1

Please check result of userrow and not check userstmt value

$usersql = "SELECT User, Password FROM Users WHERE User = '" . $_POST['inputUsername'] . "' AND Password = '" . $_POST['inputPassword'] . "'";
$userstmt = sqlsrv_query($conn, $usersql);
    $userrow = sqlsrv_fetch_array($userstmt, SQLSRV_FETCH_ASSOC);

if (count($userrow) > 0 ) {
    session_regenerate_id();
    $_SESSION['loggedin'] = TRUE;
    $_SESSION['name'] = $_POST['username'];
    $_SESSION['id'] = $id;
    header('Location: home.php');
}
else {
    $_SESSION['loggedin'] = FALSE;
    header('Location: login.php');

}
Mahesh Bhatnagar
  • 1,042
  • 1
  • 7
  • 8