if() not working

  • (2 Pages)
  • +
  • 1
  • 2

23 Replies - 598 Views - Last Post: 31 January 2014 - 10:59 AM Rate Topic: -----

#1 Ambitious  Icon User is offline

  • D.I.C Head

Reputation: 5
  • View blog
  • Posts: 131
  • Joined: 08-May 13

if() not working

Posted 28 January 2014 - 09:23 PM

Could someone help me?

function is_admin($user)
{
$con=mysqli_connect("localhost"user","password","db");
$rank=mysqli_query($con,"SELECT type FROM users WHERE username='".$user."'");
$row=mysqli_fetch_array($rank);
$status=intval($row['type']);
echo $status;
if($status<'5')
{
header('Locaton: noaccess.php');}}



The value of $status is 3.
But it doesn't redirect at all for some reason.
I'm testing it with this
<?php
include 'core/init.php';
include 'core/functions/CheckPermissions.php';
is_admin($user_data['username']);
?>


Can someone help please?

This post has been edited by Ambitious: 28 January 2014 - 09:24 PM


Is This A Good Question/Topic? 0
  • +

Replies To: if() not working

#2 CTphpnwb  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 2891
  • View blog
  • Posts: 10,022
  • Joined: 08-August 08

Re: if() not working

Posted 28 January 2014 - 09:59 PM

First, you're using user supplied data directly in a query. DON'T!!! Use prepared statements.
Next, you need distinguish between numbers and strings.

Third, you really should choose and use an indent style.
Was This Post Helpful? 0
  • +
  • -

#3 Dormilich  Icon User is offline

  • 痛覚残留
  • member icon

Reputation: 3485
  • View blog
  • Posts: 10,045
  • Joined: 08-June 10

Re: if() not working

Posted 28 January 2014 - 11:43 PM

Quote

But it doesn't redirect at all for some reason.

enable (all) error reporting and the system will tell you why it is not redirecting.
Was This Post Helpful? 0
  • +
  • -

#4 ArtificialSoldier  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 325
  • View blog
  • Posts: 1,202
  • Joined: 15-January 14

Re: if() not working

Posted 29 January 2014 - 09:56 AM

If you echo the status, or anything else for that matter (HTML, whitespace, etc), then sending headers will not work. You can only send a header before you've sent any other output, because once you send any output the server will immediately send all headers. Trying to send another header after output has already started will not work.
Was This Post Helpful? 0
  • +
  • -

#5 ChubbyNinja  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 6
  • Joined: 06-December 13

Re: if() not working

Posted 29 January 2014 - 10:12 AM

you should replace

if($status<'5')
{
header('Locaton: noaccess.php');}}



with

if( $status < 5) // check int instead of str
{
	header('Locaton: noaccess.php');
	die(); // stop the script from executing past this point
}



so the full function would be

function is_admin($user)
{
	$con=mysqli_connect("localhost"user","password","db");
	$rank=mysqli_query($con,"SELECT type FROM users WHERE username='".$user."'");
	$row=mysqli_fetch_array($rank);
	$status=intval($row['type']);
	if( $status < 5 )
	{
		header('Locaton: noaccess.php');
		die();
	}
}


Was This Post Helpful? 0
  • +
  • -

#6 CTphpnwb  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 2891
  • View blog
  • Posts: 10,022
  • Joined: 08-August 08

Re: if() not working

Posted 29 January 2014 - 10:29 AM

View PostChubbyNinja, on 29 January 2014 - 12:12 PM, said:

so the full function would be

function is_admin($user)
{
	$con=mysqli_connect("localhost"user","password","db");
	$rank=mysqli_query($con,"SELECT type FROM users WHERE username='".$user."'");
	$row=mysqli_fetch_array($rank);
	$status=intval($row['type']);
	if( $status < 5 )
	{
		header('Locaton: noaccess.php');
		die();
	}
}


That still leaves the query open to SQL injection attack.
Was This Post Helpful? 0
  • +
  • -

#7 no2pencil  Icon User is online

  • Toubabo Koomi
  • member icon

Reputation: 5182
  • View blog
  • Posts: 26,883
  • Joined: 10-May 07

Re: if() not working

Posted 29 January 2014 - 10:43 AM

View PostChubbyNinja, on 29 January 2014 - 12:12 PM, said:

so the full function would be

function is_admin($user)
{
	$con=mysqli_connect("localhost"user","password","db");
...
}


That line works? I count an odd number of quotes, which means one isn't closed. Assuming localhost is the connection for the db, I think you mean :

$con=mysqli_connect("localhost", "user", "password", "db");

Validating $con, rather than just running with it & assuming it's connected, would have pointed this out.

Validate!
Validate!
Validate!
Was This Post Helpful? 0
  • +
  • -

#8 JackOfAllTrades  Icon User is offline

  • Saucy!
  • member icon

Reputation: 6036
  • View blog
  • Posts: 23,431
  • Joined: 23-August 08

Re: if() not working

Posted 29 January 2014 - 02:10 PM

header('Locaton: noaccess.php');


The header is Location, not Locaton.
Was This Post Helpful? 0
  • +
  • -

#9 Ambitious  Icon User is offline

  • D.I.C Head

Reputation: 5
  • View blog
  • Posts: 131
  • Joined: 08-May 13

Re: if() not working

Posted 29 January 2014 - 06:45 PM

View PostJackOfAllTrades, on 29 January 2014 - 02:10 PM, said:

header('Locaton: noaccess.php');


The header is Location, not Locaton.

Oh my goodness.
Thank you for your help everyone! I've fixed this problem that i've been struggling with for days.
Was This Post Helpful? 0
  • +
  • -

#10 ChubbyNinja  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 6
  • Joined: 06-December 13

Re: if() not working

Posted 30 January 2014 - 02:14 AM

View PostCTphpnwb, on 29 January 2014 - 10:29 AM, said:

That still leaves the query open to SQL injection attack.

You're assuming that $user is not sanitized outside of the function.

For instance, the value actually comes from a the array

is_admin($user_data['username']);


It's fair to assume that $user_data is the result of actually fetching the user from the database in the first place. And therefore is not user input & not open for SQL inj.

Having said that, yes prepared statements are recommended.

Can't believe how many of us missed the i in Location though! how embarrassing.
Was This Post Helpful? 0
  • +
  • -

#11 CTphpnwb  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 2891
  • View blog
  • Posts: 10,022
  • Joined: 08-August 08

Re: if() not working

Posted 30 January 2014 - 05:45 AM

View PostChubbyNinja, on 30 January 2014 - 04:14 AM, said:

View PostCTphpnwb, on 29 January 2014 - 10:29 AM, said:

That still leaves the query open to SQL injection attack.

You're assuming that $user is not sanitized outside of the function.

Partly, yes.
On the one hand I'm assuming that because the function was defined it may be called more than once from more than one location and the data passed to it can come from anywhere.
On the other I see that even if the current version of the code only passes data that comes from the database, that data might contain SQL commands. Remember that prepared statements don't scrub the data at all. They simply separate it from the actual query, so an attacker might manage to store an SQL attack in the username itself.
Was This Post Helpful? 2
  • +
  • -

#12 ArtificialSoldier  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 325
  • View blog
  • Posts: 1,202
  • Joined: 15-January 14

Re: if() not working

Posted 30 January 2014 - 09:18 AM

Quote

It's fair to assume that $user_data is the result of actually fetching the user from the database in the first place. And therefore is not user input & not open for SQL inj.

Just because it's already in the database doesn't mean it doesn't need to always be escaped. Maybe it actually does contain malicious characters and was properly escaped the first time it was added, so nothing happened. If you trust it after that point and don't bother escaping it every time you put it in a query then eventually it's going to catch you.
Was This Post Helpful? 3
  • +
  • -

#13 CTphpnwb  Icon User is online

  • D.I.C Lover
  • member icon

Reputation: 2891
  • View blog
  • Posts: 10,022
  • Joined: 08-August 08

Re: if() not working

Posted 30 January 2014 - 10:16 AM

View PostArtificialSoldier, on 30 January 2014 - 11:18 AM, said:

Just because it's already in the database doesn't mean it doesn't need to always be escaped.

My second point, but you said it better. :wink:

It's best to never treat data of any kind and from any source as a query. Prepare the query and then add the data. The one time you don't will probably be when you run into trouble.
Was This Post Helpful? 0
  • +
  • -

#14 ChubbyNinja  Icon User is offline

  • New D.I.C Head

Reputation: 1
  • View blog
  • Posts: 6
  • Joined: 06-December 13

Re: if() not working

Posted 31 January 2014 - 07:02 AM

very good points :-)
Was This Post Helpful? 0
  • +
  • -

#15 Atli  Icon User is offline

  • D.I.C Lover
  • member icon

Reputation: 3712
  • View blog
  • Posts: 5,963
  • Joined: 08-June 10

Re: if() not working

Posted 31 January 2014 - 07:39 AM

I feel somebody should point out that comparing the number 5 to the string '5' works just fine in PHP. It'll convert it implicitly so the string actually is compared as a number.

Though I'll agree that specifically converting the left hand side of the comparison to a number and then specifying a string for the right hand side is a little odd.
Was This Post Helpful? 0
  • +
  • -

  • (2 Pages)
  • +
  • 1
  • 2