Ban Function is not working

[Tom8001]

I am trying to make a ban function and i have the users id being passed through the URL and i can't seem to get it working.

<?php

require 'connect.php';

include 'header.php';

error_reporting(E_ALL | E_NOTICE);

session_start();

$ban = "SELECT id FROM $tbl_name WHERE id={$_GET['id']} AND active='$active'";
$banres = $con->query($ban);
$row = $con->fetch_assoc($banres);
$active = $row['active'];

if($banres) {

	$active_update = "UPDATE $tbl_name SET active=0";

}

?>

[NotionCommotion]

Display all errors and notices.  Do you get any errors?  Are $tbl_name and $active defined?  Do you really want if($banres)?  Escape your user inputs.  Use PDO.

[ginerjm]

You need to turn on display errors to see any possible faults in your code.

 

As for what you want to do, I don't see it here.  You write a query using values that 'we' have no idea about, you run this query but don't check if it succeeds before grabbing results from it.  Basically we don't see what's wrong here, nor do you.

[Tom8001]

Sorry i thought i added the error i get 

Notice: Undefined index: id in /home/www/ps3modding.co.uk/webdir/banUser.php on line 11

Notice: Undefined variable: active in /home/www/ps3modding.co.uk/webdir/banUser.php on line 11

Fatal error: Call to undefined method mysqli::fetch_assoc() in /home/www/ps3modding.co.uk/webdir/banUser.php on line 13

Edited December 15, 2014 by Tom8001

[Tom8001]

Display all errors and notices.  Do you get any errors?  Are $tbl_name and $active defined?  Do you really want if($banres)?  Escape your user inputs.  Use PDO.

$tbl_name is defined in connect.php and $active is stored in a session when the user logs in but i want to change active in the table to 0 so they are banned and i have used $_GET['id'] because i am passing the id through the URL so what i really want is to select the user id from the url and change active to 0,

and if($banres) was originally if($banres > 0) 

[ginerjm]

Apparently not.  If you are getting the error somethings not defined.

[Barand]

1 ) fetch_assoc() is a mysqli_result method, so you need to use

 

          $row = $banres->fetch_assoc();

 

2 ) Also, to access $row['active'] you need to select 'active' in your query. You only select the id.

 

3 ) Use a prepared query or escape the GET value - don't use it directly in your query

[Tom8001]

It says active is undefined when 

$active = $row['active'];

[Tom8001]

I have defined active and selected it now the only error i am getting is 

Fatal error: Call to a member function fetch_assoc() on a non-object in /home/www/ps3modding.co.uk/webdir/banUser.php on line 13

[ginerjm]

Means your connection is not good or you are not using the 'connected' variable to access the function.

 

Show us the code

[Tom8001]

Means your connection is not good or you are not using the 'connected' variable to access the function.

 

Show us the code

In my query i to define the table name i am using $tbl_name which is in the connect.php file could that be the problem?

[Barand]

echo $ban;

post the results from that so we can see if the query looks right

[Tom8001]

<?php

require 'connect.php';

include 'header.php';

error_reporting(E_ALL | E_NOTICE);
ini_set('display_errors', '1');

session_start();

echo "<title> Ban ".$_GET['user']." </title>";

echo "Account selected: <font color='red'><b>".$_GET['user']."</b></font> The account's id is <font color='red'><b> ".$_GET['id']."</b></font> ";

$query = $con->query("SELECT id,active FROM webdir_usr");
$active = $query->fetch_array();
echo $query;

	if($active == 1) {

	$active_update = "UPDATE $tbl_name SET $active=0";

	} else {
	
	throw new Exception("There was an error in the query");
	
	}

?>

When i try to echo the query i get 

Catchable fatal error: Object of class mysqli_result could not be converted to string in /home/www/ps3modding.co.uk/webdir/banUser.php on line 18

[adam_bray]

You're using $active wrong -

$active = $query->fetch_array(); // $active is now an array
if($active == 1) // $active is an array, it will never == 1
$active_update = "UPDATE $tbl_name SET $active=0"; // use $active as a string

What are you trying to update at the end?

 

For your current error you need to change -

echo $query;
// change to (in the same place)
print_r($active);

You're also going to get warnings about the $_GETs because they may not be defined -

$user = (isset($_GET['user']))? $_GET['user'] : 'invalid';
$user_id = (isset($_GET['id']))? $_GET['id'] : 0;

if($user == 'invalid'] || $user_id == 0) {
// error code
}

Edited December 16, 2014 by adam_bray

[Tom8001]

 

You're using $active wrong -

$active = $query->fetch_array(); // $active is now an array
if($active == 1) // $active is an array, it will never == 1
$active_update = "UPDATE $tbl_name SET $active=0"; // use $active as a string

What are you trying to update at the end?

 

For your current error you need to change -

echo $query;
// change to (in the same place)
print_r($active);

You're also going to get warnings about the $_GETs because they may not be defined -

$user = (isset($_GET['user']))? $_GET['user'] : 'invalid';
$user_id = (isset($_GET['id']))? $_GET['id'] : 0;

if($user == 'invalid'] || $user_id == 0) {
// error code
}

The gets are fine they are being passed through the url

[Tom8001]

Nvm i got it to work by doing 

<?php

require 'connect.php';

include 'header.php';

error_reporting(E_ALL | E_NOTICE);
ini_set('display_errors', '1');

session_start();

echo "<title> Ban ".$_GET['user']." </title>";

echo "Account selected: <font color='red'><b>".$_GET['user']."</b></font> The account's id is <font color='red'><b> ".$_GET['id']."</b></font> ";

$sql = "UPDATE $tbl_name SET active=0 WHERE id={$_GET['id']}";

		if ($con->query($sql) === TRUE) {
  		  echo "<font face='Tahoma' color='blue'> ".$_GET['user']." has been banned! </font>";
		} else {
 	   echo "Error updating record: " . $con->error;
	}

?>

[ginerjm]

Glad you got it working.  Now you need to work on your html and stop using long-ago deprecated attributes and practice secure procedures.

 

no more font tag

filter your input before using it and before echoing it in your output.

[Jacques1]

Speaking of security: Your code has none.

• You allow SQL injection. Never heard of Little Bobby Tables?
• You allow HTML/JavaScript injection. Never heard of escaping?
• Where do you check if the current user actually has permissions to ban? Or can everbody do that?
• Changing data in a normal GET request is fundamentally wrong and a very, very bad idea. For example, if I put a picture into this forum with the URL https://www.yoursite.com/banUser.php?user=123, then you'll automatically ban user 123 merely by visiting phpfreaks. Maybe I could even make you ban yourself.
• You show all your internal PHP errors on the website. This is great info for attackers but very irritating for legitimate users.

Unless you're writing a demo application to teach people how to “hack” a website, you're doing it wrong. Has it never occured to you that you need to protect your application? Not every user of the Internet plays nice, you know?

Edited December 17, 2014 by Jacques1

[Tom8001]

Speaking of security: Your code has none.

• You allow SQL injection. Never heard of Little Bobby Tables?
• You allow HTML/JavaScript injection. Never heard of escaping?
• Where do you check if the current user actually has permissions to ban? Or can everbody do that?
• Changing data in a normal GET request is fundamentally wrong and a very, very bad idea. For example, if I put a picture into this forum with the URL https://www.yoursite.com/banUser.php?user=123, then you'll automatically ban user 123 merely by visiting phpfreaks. Maybe I could even make you ban yourself.
• You show all your internal PHP errors on the website. This is great info for attackers but very irritating for legitimate users.

Unless you're writing a demo application to teach people how to “hack” a website, you're doing it wrong. Has it never occured to you that you need to protect your application? Not every user of the Internet plays nice, you know?

The SQL Injection was a mistake i usually add that in but i forgot, and the permissions are set in another page. & I have added a script so that administrators cannot edit themselves.

[Tom8001]

It has stopped working since i added 

$sql = "UPDATE $tbl_name SET active=0 WHERE id={".mysqli_real_escape_string($con, $_GET['id'])."}";

I don't get an error but it just doesn't execute properly I'm guessing it's the escape string part that is causing it.

[Jacques1]

The braces are wrong, and escaping is entirely useless if you don't quote the value.

 

Manual escaping is actually a rather poor solution, because people constantly forget it (as you just saw) or make some mistake (as you just saw). Better use prepared statements and stop inserting values into query strings altogether.

[Tom8001]

The braces are wrong, and escaping is entirely useless if you don't quote the value.

 

Manual escaping is actually a rather poor solution, because people constantly forget it (as you just saw) or make some mistake (as you just saw). Better use prepared statements and stop inserting values into query strings altogether.

I've never done it using prepare I'm still learning mysqli and don't understand parts of it.

[Jacques1]

The link contains several examples which you can copy and try out. If you have concrete questions, feel free to ask them.

[hansford]

session_start();

This should to be called at the top of the page before any output to the browser.

There is a default buffer size set in php.ini, but it's different depending on which version you're using.

So you may get away with echoing output prior to session_start(), but why risk it and it's simply a poor practice.

Also, error_reporting should be called before any other PHP code is executed as well. Get in the practice of putting all of that code at the top of the page and not strewn all over here and there. 

Edited December 18, 2014 by hansford

[Tom8001]

The link contains several examples which you can copy and try out. If you have concrete questions, feel free to ask them.

Thanks