Invalid Request to service file falls through

[Karaethon]

My app just found an error in my login.php file on accident. The app is supposed to get credentials from the user then submit them to my server for verification. I accidentally submitted null credentials (empty strings) and the server returned auth=true. I can't figure out why. 

Server code:

<?php
require_once '../../includes/db_connect.inc';
require_once 'user.php';
$header = "Content-Type: application/json";
$_SESSION['error'] = array();
if (!isset($_GET['secureSubmit']) || $_GET['secureSubmit'] != true){
    die(header("Location: ../access_denied.php"));
}
// check requirements
$required = array('username', 'password');
foreach ($required as $requiredField){
  if (!isset($_GET[$requiredField]) || $_GET[$requiredField] == ""){
    $_SESSION['error'][] = $requiredField . " is incomplete or missing.";
    }
}
  if (count($_SESSION['error']) > 0){
    $errors = array();
    for ($i = 0; $i < count($_SESSION['error']); $i++){
      $errors[]=array(
        'num' => $i,
        'desc' => $_SESSION['error'][$i]
      );
  }
  print json_encode($errors);
  exit;
}else{
  $user = new User;
  if ($user->authenticate($_GET['username'], $_GET['password'])){
    print json_encode(array(
      'auth' => true,
      'call' => "login",
      'sid' => session_id(),
      'credits' => $_SESSION['credits']
      )
    );
  }else{
    print json_encode(array('auth' => false,));
  }
}
?>
	

I went to my browser and entered

http://localhost:10509/login.php?secureSubmit=true&username=%22%22&password=%22%22

and recieved

{auth":true,"call":"login","sid":"29e81981a4709407e2fd8a8f734ad9bc","credits":null}

as a response, can anyone find where the invalid positive is coming from? The initial check for empty username or password should be failing before it ever gets to the check.

Edited March 19, 2019 by Karaethon
Code cleanup for readability

[gw1500se]

I would say it is a bug in your "User" class, which you didn't post.

[mac_gyver]

for the example you posted, they are not empty values. they consist of two double-quotes each. empty values would be - username=&password=. of concern would be why your ->authenticate() method call is returning a true value for a username equal to a string consisting of "" and a password equal to a string consisting of ""

also, your use and test of $_GET['secureSubmit'] will be true as long as there's any non-empty/non-zero value in the url. the =true in the url is a string consisting of the letters - t,r,u, and e. only secureSubmit=&... and secureSubmit=0&... would fail the logic test.

Edited March 19, 2019 by mac_gyver

[Karaethon]

4 minutes ago, mac_gyver said:

for the example you posted, they are not empty values. they consist of two double-quotes each. empty values would be - username=&password=. of concern would be why your ->authenticate() method call is returning a true value for a username equal to "" and a password equal to ""

also, your use and test of $_GET['secureSubmit'] will be true as long as there's any non-empty/non-zero value in the url. the =true in the url is a string consisting of the letters - t,r,u, and e. only secureSubmit=&... and secureSubmit=0&... would fail the logic test.

crap, i didnt even think to make sure its a string "true" instead of boolean true. and I'll look into why user->authenticate is returning true. Thank you both for your replies.

[Karaethon]

hmmm. Im not sure why. User:

	<?php
	class User {
    public $id;
    public $username;
    public $email;
    public $fname;
    public $lname;
    public $addr1;
    public $addr2;
    public $city;
    public $state;
    public $zip;
    public $phone;
    public $ssn;
    public $dob;
    public $paypal;
    public $credits;
    public $winnings;
    public $loggedIn = false;
	    function __construct(){
        if (session_id() == ""){
            session_start();
        }
        if(isset($_SESSION['loggedIn']) && $_SESSION['loggedIn'] == true){
            $this -> _initUser();
        }
    }//end __construct
	    public function authenticate($user, $pass){
        $mysqli = new mysqli(DBHOST, DBUSER, DBPASS, DBNAME);
        if ($mysqli->connect_errno){
            die("Cannot connect to MySQL: " . $mysqli->connect_error);
            return false;
        }
        $safeUser = $mysqli->real_escape_string($user);
        $incomingPassword = $mysqli->real_escape_string($pass);
        $query = "SELECT* FROM Players WHERE Username = '{$safeUser}'";
        if(!$result = mysqli_query($mysqli, $query)){
            die("Cannot retrieve accoutnt for {$user}");
            return false;
        }
        $row = $result->fetch_assoc();
        $dbPassword = $row['Password'];
        if (password_verify($incomingPassword, $dbPassword) != $dbPassword){
            die("Password for {$user} does not match stored password.");
            return false;
        }
        $this->id = $row['Player_ID'];
        $this->username = $row['Username'];
        $this->email = $row['Email'];
        $this->fname = $row['FName'];
        $this->lname = $row['LName'];
        $this->addr1 = $row['Addr1'];
        $this->addr2 = $row['Addr2'];
        $this->city = $row['City'];
        $this->state = $row['State'];
        $this->zip = $row['Zip'];
        $this->phone = $row['Phone'];
        $this->ssn = $row['SSN'];
        $this->dob = $row['DoB'];
        $this->paypal = $row['PayPal'];
        $this->credits = $row['Credits'];
        $this->winnings = $row['YTD_Winnings'];
        $this->loggedIn = true;
        $this->_setSession();
        return true;
    } //end authenticate function
	    	public function logout(){
		$this -> loggedIn = false;
		if (session_id() == ''){
			session_start();
		}
		$_SESSION['loggedIn'] = false;
		foreach ($_SESSION as $key => $value){
			$_SESION[$key] = "";
			unset($_SESSION[$key]);
		}
		$_SESSION = array();
		if (ini_get("session.use_cookies")){
			$cookieParameters = session_get_cookie_params();
			setcookie(session_name(), "", time() - 86400, $cookieParameters['path'],$cookieParameters['domain'], $cookieParameters['secure'], $cookieParameters['httponly']);
		} //end if
		session_destroy();
	}// end logout function

	private function _setSession(){
		if (session_id() == ''){
			session_start();
		}
		$_SESSION['id'] = $this->id;
		$_SESSION['username'] = $this->username;
		$_SESSION['email'] = $this->email;
		$_SESSION['fname'] = $this->fname;
		$_SESSION['lname'] = $this->lname;
		$_SESSION['addr1'] = $this->addr1;
		$_SESSION['addr2'] = $this->addr2;
		$_SESSION['city'] = $this->city;
		$_SESSION['state'] = $this->state;
		$_SESSION['zip'] = $this->zip;
		$_SESSION['phone'] = $this->phone;
		$_SESSION['ssn'] = $this->ssn;
		$_SESSION['dob'] = $this->dob;
		$_SESSION['paypal'] = $this->paypal;
		$_SESSION['credits'] = $this->credits;
		$_SESSION['winnings'] = $this->winnings;
		$_SESSION['loggedIn'] = $this->loggedIn;
	}//end setsession function

	private function _initUser(){
		if (session_id() == ''){
			session_start();
		}
		$this->id = $_SESSION['id'];
		$this->username = $_SESSION['username'];
		$this->email = $_SESSION['email'];
		$this->fname = $_SESSION['fname'];
		$this->lname = $_SESSION['lname'];
		$this->addr1 = $_SESSION['addr1'];
		$this->addr2 = $_SESSION['addr2'];
		$this->city = $_SESSION['city'];
		$this->state = $_SESSION['state'];
		$this->zip = $_SESSION['zip'];
		$this->phone = $_SESSION['phone'];
		$this->ssn = $_SESSION['ssn'];
		$this->dob = $_SESSION['dob'];
		$this->paypal = $_SESSION['paypal'];
		$this->credits = $_SESSION['credits'];
		$this->winnings = $_SESSION['winnings'];
		$this->loggedIn = $_SESSION['loggedIn'];
	}//end initUser function
}//end user class
?>

Edited March 19, 2019 by Karaethon
user class didn't all paste

[gw1500se]

You are not checking if the query returned anything.

if(!$result = mysqli_query($mysqli, $query)){

Checks if the query failed, not if it returned no results. It appears to me that if the incoming password is null, it will indeed match the null dbpassword.

[Karaethon]

11 minutes ago, gw1500se said:

You are not checking if the query returned anything.

if(!$result = mysqli_query($mysqli, $query)){

Checks if the query failed, not if it returned no results. It appears to me that if the incoming password is null, it will indeed match the null dbpassword.

that may be it, i just added a line to the output array ( "playerID" => $_SESSION['id'] ) and it returned null for the id.

....

so what's the code to do that? mysqli_???

num_rows?

Edited March 19, 2019 by Karaethon
ask a question

[mac_gyver]

15 minutes ago, Karaethon said:

if (password_verify($incomingPassword, $dbPassword) != $dbPassword)

the above logic is a problem unto itself. password_verify() returns a boolean true/false value. the above logic will change operation depending on what $dbPassword starts with. remove the !=$dbPassword from that line.

[Karaethon]

2 minutes ago, mac_gyver said:

the above logic is a problem unto itself. password_verify() returns a boolean true/false value. the above logic will change operation depending on what $dbPassword starts with. remove the !=$dbPassword from that line.

oops, I converted that line to use password_verify() from one that used crypt()

[Karaethon]

WOOT! It's working correctly now, my eternal gratitude to both of you for the help and knowledge. 

 

So, Mac, I noticed your Avatar is the gatecode for earth, with Mac_Gyver as you username is that a nod to both stargate and macgyver staring the same actor? if so, cool, both shows rock.

Edited March 19, 2019 by Karaethon
tangental thought

[mac_gyver]

10 minutes ago, Karaethon said:

So, Mac, I noticed your Avatar is the gatecode for earth, with Mac_Gyver as you username is that a nod to both stargate and macgyver staring the same actor? if so, cool, both shows rock.

yes.