-2

SO i have been trying with a php project and everything is working fine.Except a bit extra. Login page redirects to Dashboard even with incorrect details .So basically login is bypassed regardless the login details. Also By putting "sitename/dashboard" directly also bypasses the login. Below Are my Code.

1.index(login page)

<?php
require('inc/dbPlayer.php');
require('inc/sessionManager.php');
$msg="";
if ($_SERVER["REQUEST_METHOD"] == "POST") {

    if (isset($_POST["btnLogin"])) {

        $db = new \dbPlayer\dbPlayer();
        $msg = $db->open();

        if ($msg == "true") {
            $userPass = md5("hms2015".$_POST['password']);
            $loginId = $_POST["email"];
            $query = "select loginId,userGroupId,password,name,userId from users where loginId='" . $loginId . "' and password='" . $userPass . "';";
            var_dump($query);
            $result = $db->getData($query);
            //var_dump($result);
            $info = array();
            while ($row = mysql_fetch_assoc($result)) {

                array_push($info, $row['loginId']);
                array_push($info, $row['userGroupId']);
                array_push($info, $row['password']);
                array_push($info, $row['name']);
                array_push($info, $row['userId']);

            }
            //$db->close();
            $ses = new \sessionManager\sessionManager();
   $ses->start();
                $ses->Set("loginId", $info[0]);
                $ses->Set("userGroupId", $info[1]);
                $ses->Set("name", $info[3]);
                $ses->Set("userIdLoged", $info[4]);
            if (is_null($info[0])) {
                $msg = "Login Id or Password Wrong!";

            }
            else
            {
                
            }
            if($info[1]=="UG004")
            {
                header('Location: http://localhost/hms/sdashboard.php');
            }
            elseif($info[1]=="UG003")
            {

                header('Location: http://localhost/hms/edashboard.php');
            }
            else
            {
                header('Location: http://localhost/hms/dashboard.php');
            }


        }

    }
}
?>
<!DOCTYPE html>
<html lang="en">
<head>
    <title>HMS</title>

</head>
<body>
<div class="container">
    <div class="row">

        <div class="col-md-4 col-md-offset-4">
                <div class="panel-body">
                    <form name="login" action="index.php" accept-charset="utf-8" method="post" enctype="multipart/form-data">
                        <fieldset>
                            <div class="form-group">
                                <input class="form-control" placeholder="E-mail/Login ID" name="email" type="text" autofocus required>
                            </div>
                            <div class="form-group">
                                <input class="form-control" placeholder="Password" name="password" type="password" value="" required>
                            </div>
                            <div class="checkbox">
                                <label>
                                    <input name="remember" type="checkbox" value="Remember Me">Remember Me
                                </label>
                                <a href="#" class="red pull-right">Forget Password</a>
                                <label id="loginMsg" class="red"><?php echo $msg ?></label>
                            </div>
                            
                            <button type="submit" name="btnLogin" class="btn btn-lg btn-success btn-block"><i class="glyphicon glyphicon-log-in"></i> Login</button>
                        </fieldset>
                    </form>
                </div>
            </div>
        </div>
    </div>
</div>
</body>

</html>

2.dbplayer

<?php
namespace dbPlayer;


class dbPlayer {

    private $db_host="localhost";
    private $db_name="hms";
    private $db_user="root";
    private $db_pass="";
    protected $con;

   public function open(){
        $con = mysql_connect($this->db_host,$this->db_user,$this->db_pass);
       if($con)
       {
           $dbSelect = mysql_select_db($this->db_name);

           if($dbSelect)
           {
               return "true";
           }
           else
           {
               return mysql_error();
           }

       }
        else
        {
            return  mysql_error();
        }

    }
    public  function close()
    {
        $res=mysql_close($this->con);
        if($res)
        {
            return "true";
        }
        else
        {
            return mysql_error();
        }

    }

    public function insertData($table,$data)
    {
        $keys   = "`" . implode("`, `", array_keys($data)) . "`";
        $values = "'" . implode("', '", $data) . "'";
       //var_dump("INSERT INTO `{$table}` ({$keys}) VALUES ({$values})");
        mysql_query("INSERT INTO `{$table}` ({$keys}) VALUES ({$values})");

        return mysql_insert_id().mysql_error();

    }
    public function registration($query,$query2)
    {
        $res=mysql_query($query);
        if($res)
        {

            $res=mysql_query($query2);
            if($res)
            {

                return "true";
            }
            else
            {
                return mysql_error();
            }

        }
        else
        {
            return mysql_error();
        }


    }
    public  function  getData($query)
    {
        $res = mysql_query($query);
        if(!$res)
        {
            return "Can't get data ".mysql_error();
        }
        else
        {
            return $res;
        }

    }
    public function  update($query)
    {
        $res = mysql_query($query);
        if(!$res)
        {
            return "Can't update data ".mysql_error();
        }
        else
        {
            return "true";
        }
    }
    public  function  updateData($table,$conColumn,$conValue,$data)
    {
        $updates=array();
        if (count($data) > 0) {
            foreach ($data as $key => $value) {

                $value = mysql_real_escape_string($value); // this is dedicated to @Jon
                $value = "'$value'";
                $updates[] = "$key = $value";
            }
        }
        $implodeArray = implode(', ', $updates);
        $query ="UPDATE ".$table." SET ".$implodeArray." WHERE ".$conColumn."='".$conValue."'";
       //var_dump($query);
        $res = mysql_query($query);
        if(!$res)
        {
            return "Can't Update data ".mysql_error();
        }
        else
        {
            return "true";
        }
    }

    public  function delete($query)
    {
        $res = mysql_query($query);
       // var_dump($query);
        if(!$res)
        {
            return "Can't delete data ".mysql_error();
        }
        else
        {
            return "true";
        }
    }

    public  function  getAutoId($prefix)
    {
        $uId="";
        $q = "select number from auto_id where prefix='".$prefix."';";
        $result = $this->getData($q);
        $userId=array();
        while($row = mysql_fetch_assoc($result))
        {

            array_push($userId,$row['number']);

        }
        // var_dump($UserId);
        if(strlen($userId[0])>=1)
        {
            $uId=$prefix."00".$userId[0];
        }
        elseif(strlen($userId[0])==2)
        {
            $uId=$prefix."0".$userId[0];
        }
        else
        {
            $uId=$prefix.$userId[0];
        }
        array_push($userId,$uId);
        return $userId;

    }
    public  function  updateAutoId($value,$prefix)
    {
         $id =intval($value)+1;

        $query="UPDATE auto_id set number=".$id." where prefix='".$prefix."';";
        return $this->update($query);

    }

    public  function execNonQuery($query)
    {
        $res = mysql_query($query);
        if(!$res)
        {
            return "Can't Execute Query".mysql_error();
        }
        else
        {
            return "true";
        }
    }
    public  function execDataTable($query)
    {
        $res = mysql_query($query);
        if(!$res)
        {
            return "Can't Execute Query".mysql_error();
        }
        else
        {
            return $res;
        }
    }

}

3.Session manager

<?php
namespace sessionManager;


class sessionManager {

    public function Set($key,$value)
    {

        $_SESSION[$key] = $value;
       // $_SESSION['start'] = time();
       // $_SESSION['expire'] = $_SESSION['start'] + (30 * 60);
    }
    public function Get($key)
    {

       // session_start();
        if(isset($_SESSION[$key])) {
            return $_SESSION[$key];
        }
        else
        {
            return null;
        }

    }
     public function isExpired()
    {
        //session_start();
        $now = time();
        if ($now > $_SESSION['expire']) {
            session_unset();
            session_destroy();
            return true;
        }
        else
        {
            return false;
        }
    }
    public function remove($key)
    {
        //session_start();
        unset($_SESSION[$key]);
    }
    public function  start()
    {
        session_start();
        $_SESSION['start'] = time();
        $_SESSION['expire'] = $_SESSION['start'] + (30 * 60);

    }



}
Martin
  • 22,212
  • 11
  • 70
  • 132
  • 6
    **Too much code**. You need to do a better job of troubleshooting this yourself. We are *not* debuggers. You need isolate the problem and debug from there. If you're stuck provide a clear explanation of what isn't working with a [Minimal, Complete, and Verifiable example](//stackoverflow.com/help/mcve). I suggest reading [ask] a good question and [the perfect question](http://codeblog.jonskeet.uk/2010/08/29/writing-the-perfect-question/). Also, be sure to take the [tour] and read [this](//meta.stackoverflow.com/q/347937/). – John Conde Feb 22 '19 at 12:34
  • 5
    FYI, [you shouldn't use `mysql_*` functions in new code](http://stackoverflow.com/questions/12859942/). They have been deprecated since v5.5 (Jun 2013) and removed since v7.0 (Dec 2015). See the [red box](http://php.net/manual/en/function.mysql-connect.php)? Learn about [*prepared statements*](https://en.wikipedia.org/wiki/Prepared_statement) instead, and use [PDO](http://php.net/pdo) or [MySQLi](http://php.net/mysqli) - [this article](http://php.net/manual/en/mysqlinfo.api.choosing.php) will help you decide which one is best for you. – John Conde Feb 22 '19 at 12:34
  • 5
    Please read about **[SQL injection](https://en.wikipedia.org/wiki/SQL_injection)**. Instead of building queries with string concatenation, use **[prepared statements](https://secure.php.net/manual/en/pdo.prepare.php)** with **[bound parameters](https://secure.php.net/manual/en/pdostatement.bindparam.php)**. See **[this page](https://phptherightway.com/#databases)** and **[this post](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php)** for some good examples. – John Conde Feb 22 '19 at 12:34
  • 6
    `md5()` is obsolete for hashing passwords and should *not be used*. PHP provides [password_hash()](//php.net/manual/en/function.password-hash.php) and [password_verify()](//php.net/manual/en/function.password-verify.php), please use them. And here are some [good ideas about passwords](//www.owasp.org/index.php/Password_Storage_Cheat_Sheet). If you are using a PHP version prior to 5.5 [there is a compatibility pack available here](https://github.com/ircmaxell/password_compat) (and you should consider upgrading to a supported version of PHP). – John Conde Feb 22 '19 at 12:34
  • 1
    You have no path when the form has been submitted _not_ to get redirected to a dashboard. You test if something is null, then redirect the user anyway... – Jonnix Feb 22 '19 at 12:37
  • 2
    This is a great time for you to start learning how to debug. A Google search for something like "how to debug PHP" will get you started. If you're using an IDE, it probably has helpful debugging tools. Even if you're not, you can still do some basic debugging by outputting key values throughout your code, running it, and observing that output. Use this process to narrow down the problem. If your code is doing something you don't expect, then somewhere in your code a specific operation is producing a result you didn't expect. Find that operation, determine the values at the time. – David Feb 22 '19 at 12:37
  • Thanks guys. i will try to improve my codes as per your suggestions. – Arindam Deka Feb 23 '19 at 15:47

2 Answers2

0

A few hints:

  • require values should not be in brackets.
  • you should NOT be using mysql_ functions, this library is now CEASED and unavailable in PHP 7. Get up to date to 2012 and use mysqli_ or PDO. (Why?)
  • You should be using PHP 7. As a minimum. (Why?)
  • Do NOT use md5 for hashing passwords. Use PHP's built in password_hash() function(s). (How?)
  • STOP outputting errors to screen (aka return mysql_error();). You should be sending errors to an error log (error_log(print_r(mysql_error(),true));) so the public can't see the details of the error.
  • Read your PHP Error Log. What does it say?
  • Use Prepared Statements on your database interactions. ([How?(https://phpdelusions.net/mysqli))

  • Header("Location: ... "); functions should always be immediately followed by exit;/die();

  • NEVER trust user input. Even if the user tells you it's harmless. (Why?)
  • Read your PHP Error Log. What does it say?
  • Your classes should probably have class __constuct() functions. (why?)
  • You can use Boolean Values instead of strings; use return true; instead of return "true";
  • You STILL should NOT be using mysql_ functions, Why are you still using them? Stop reading this and update your codebase! Use mysqli_ or PDO. (Why?)
  • Learn the differences between the different PHP Comparison Operators. And apply what you learn to your code.
  • Use the PHP Manual to find out and use the multitude of functions available in PHP.
  • Please get in touch with me if you wish to purchase a copy of PHP 6 (rated 4.5/5 stars on TripAdvisor).

You have a lot of reading to do, and a lot to learn. I would say good luck, but you don't need any luck, you need to read and commit yourself to learning how to use PHP properly.

Have fun.

Martin
  • 22,212
  • 11
  • 70
  • 132
  • Thanks. i will try to improve my codes. but for now i have to mention that this is an open source code example from a project and doesn't require much fixings for Sql injection as there would be no practical use of these projects apart from learning (my).i really appreciate you guys for the support. thanks again – Arindam Deka Feb 23 '19 at 15:49
  • @ArindamDeka if my answer is hepful please press the Up arrow on the Lefthand side; if my answer is the best answer and solves your issue please tick the green arrow to indicate this, Many thanks. – Martin Feb 23 '19 at 18:04
-1

You need to apply a condition whether you have record in database or not. If not then you need to bypass to login page. Change this code as below:

if ($msg == "true") {
            $userPass = md5("hms2015".$_POST['password']);
            $loginId = $_POST["email"];
            $query = "select loginId,userGroupId,password,name,userId from users where loginId='" . $loginId . "' and password='" . $userPass . "';";
            var_dump($query);
            $result = $db->getData($query);
            //var_dump($result);

            if (mysql_num_rows($result) > 0) { // means user is logged in
            $info = array();
            while ($row = mysql_fetch_assoc($result)) {

                array_push($info, $row['loginId']);
                array_push($info, $row['userGroupId']);
                array_push($info, $row['password']);
                array_push($info, $row['name']);
                array_push($info, $row['userId']);

            }
            //$db->close();
            $ses = new \sessionManager\sessionManager();
            $ses->start();
                $ses->Set("loginId", $info[0]);
                $ses->Set("userGroupId", $info[1]);
                $ses->Set("name", $info[3]);
                $ses->Set("userIdLoged", $info[4]);
            if (is_null($info[0])) {
                $msg = "Login Id or Password Wrong!";

            }
            else
            {

            }
            if($info[1]=="UG004")
            {
                header('Location: http://localhost/hms/sdashboard.php');
            }
            elseif($info[1]=="UG003")
            {

                header('Location: http://localhost/hms/edashboard.php');
            }
            else
            {
                header('Location: http://localhost/hms/dashboard.php');
            }
            }
        }

But I will suggest you to use PDO as mysql is deprecated already. Also your code is widely open for SQL injection as well so read about it as well. Hope it helps you but make your code reliable.

Rohit Mittal
  • 2,064
  • 2
  • 8
  • 18