PHP check DB if SESSION user is "banned" doesn't return true?

Question

I'm lost, see the code below. It's supposed to PHP echo "You're banned" if a record exist in db+table web_wan.blockaccount where the column name "isban" equals 1. However when testing, adding a row in the table:

id - username - isban

1 myuser 1

Nothing happens, site loads normally. I checked connection it's fine + finding column. Any ideas? SESSION should also work, otherwise I wouldn't be able to visit the page at all.

<?php
$query = $con->query("select id from web_wan.blockaccount where isban='1' and username ='.$_SESSION[username].'") or die($con->error);
while($row = $query->fetch_array())
{
    if(!empty($row))
    echo "Your Account denied access to this site!";
    exit;
} 

?>

PS. I'm new to PHP.

`$_SESSION[username]` should be `$_SESSION['username']`, as a start (and I hope you don't allow `'` in usernames...). Print out the results of your query, and check for MySQL errors - chances are the query's just silently failing because you don't check that it worked at all. — ceejayoz, Oct 29 '18 at 12:36
As your in a string already (double quotes), you don't need to concatenate anything, so the `.`'s in `'.$_SESSION[username].'` will be part of the string your searching for. Remove the `.`'s or look into prepared statements. — Nigel Ren, Oct 29 '18 at 12:38
This should probably not use a while loop to begin with ... check if the number of records returned is > 0, and draw your conclusions from that instead. — misorude, Oct 29 '18 at 12:40
Somewhat offtopic, you dont need to fetch the row's values from the database, you just want to know if a line exists: `if( $query->num_rows!==0 )` — Martijn, Oct 29 '18 at 12:42
You should use prepared statements **always** when working with a database no excuses.. Without you can bypass this ban filter or use other SQL injections when the hoster is sharing the sessions files between multiple hosts in one directory this is called Session poisoning between shared hosts.. Never trust the $_SESSION array to be safe.. — Raymond Nijland, Oct 29 '18 at 12:47

score 2 · Answer 1 · answered Oct 29 '18 at 12:39

2

Whenever you are dealing with PHP sessions, the PHP document MUST start with the following:

<?php
session_start();
//any other PHP code

The session_start(); must be called before any other PHP code is executed.

answered Oct 29 '18 at 12:39

Justin T.

826
1
5
13

score 1 · Answer 2 · answered Oct 29 '18 at 12:39

1

Use curly brackets around that if statement. If you don't only the next line runs as part of the if, exit will happen either way.

if(!empty($row))
    echo "Your Account denied access to this site!"; // only this if true
    exit; // this runs anyway



if(!empty($row)) {
    echo "Your Account denied access to this site!";
    exit;
} // now both only if true

And yes, make sure you started your session like Justin T said in his answer.

And you can also ditch the while loop:

$row = $query->fetch_array();

answered Oct 29 '18 at 12:39

delboy1978uk

12,118
2
21
39

Although your correct in a sense, as the logic is that your in a while loop that has found a row - therefore the user is banned, the `exit;` is always needed unless you ditch the `while()` – Nigel Ren Oct 29 '18 at 12:43
I didn't say to take away the exit? I said if he doesn't enclose an if statement in curly braces, then only the next line is counted as the inside of the if block. Thus exit will run wether true or false, so he needs braces – delboy1978uk Oct 29 '18 at 12:44
The only way it can get into the loop is if `$row = $query->fetch_array()` is not empty - which means `$row` is not empty. Therefore the `if(!empty($row)) {` is redundant as it has to be true. – Nigel Ren Oct 29 '18 at 12:45
That's why I told him to get rid of the while loop, after all, he is only fetching one record, so looping is not necessary. – delboy1978uk Oct 29 '18 at 12:50

score 0 · Answer 3 · answered Oct 29 '18 at 12:48

0

Per your suggestions I changed the code to the follow, which works perfect. I will also as someone here mentioned, make sure '' or other characters are allowed as usernames, good pointing that out thanks!

<?php
$query = $con->query("select id from web_wan.blockaccount where isban='1' and username ='$_SESSION[username]'") or die($con->error);
if( $query->num_rows!==0 ){
    echo "Your Account denied access to this site!";
    exit;
} 

?>

answered Oct 29 '18 at 12:48

Plankt0n

21
2

You should use prepared statements **always** when working with a database no excuses.. Without you can bypass this ban filter or use other SQL injections when the hoster is sharing the sessions files between multiple hosts in one directory this is called Session poisoning between shared hosts.. Never trust the $_SESSION array to be safe.. – Raymond Nijland Oct 29 '18 at 12:49
Thanks for your comment, could you point out what you mean with prepared statements? I'm new to the PHP world. Can you give me an example of my code converted to this statement method? This way I would be able to utilize the code all around my site. I'm quite good at deconstructing and reassemblin' code :) – Plankt0n Oct 29 '18 at 12:51
"I'm quite good at deconstructing and reassemblin' code" in that case It should be in here https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?noredirect=1&lq=1 both PDO and MySQLi examples. – Raymond Nijland Oct 29 '18 at 12:52
Thanks a lot Raymond! I'll def. give this a good read tonight! – Plankt0n Oct 29 '18 at 12:53

PHP check DB if SESSION user is "banned" doesn't return true?

3 Answers3