3
\$\begingroup\$

I have this script on top of all pages to check if user is admin otherwise we kick him. The problem is that I don't know if I am using double resources with a double query and results. Can I make this code to be only 1 query and 1 result? Please read the comments in code to understand:

<?php
session_start();
$identification = $_SESSION['session_name'];
include 'inc/database.php';
$query = "SELECT * FROM `members` WHERE `username` = '$identification' LIMIT 1";

// Here is the question about, we run first query to verify user

$result_1 = mysqli_query($database, $query);

// Now I am like obligated to query 2 times and get 2 results to make the While user_info $result_2 to work

$result_2 = mysqli_query($database, $query);

$privileges = mysqli_fetch_array($result_1, MYSQLI_ASSOC);
$user_level = $privileges["privs"];
if ($user_level=='ADMIN'){} 
else {
header('Location: security.php?error=missing correct privileges. user is not admin');
exit();
}

// If I set here to $result_1 it just does not work, I feel obligated to query 2 times and use more resources

while($user_info = mysqli_fetch_assoc($result_2))
{
?>
<html>
<head>
<title>Zona de Pruebas</title>
</head>
<body>
Username: <?php echo $user_info['username']?>, can be here because is admin</div>
</body>
</html>
<?php
}
?>
\$\endgroup\$

2 Answers 2

1
\$\begingroup\$

You do not need to re-fetch or use a while loop, simply reuse the array that you fetched the first time.

<?php
session_start();
$identification = $_SESSION['session_name'];
include 'inc/database.php';
$query = "SELECT * FROM `members` WHERE `username` = '$identification' LIMIT 1";

$result = mysqli_query($database, $query);

$user_info = mysqli_fetch_array($result, MYSQLI_ASSOC);
$user_level = $user_info["privs"];
if ($user_level=='ADMIN'){} 
else {
  header('Location: security.php?error=missing correct privileges. user is not admin');
  exit();
}

?>
<html>
<head>
<title>Zona de Pruebas</title>
</head>
<body>
Username: <?php echo $user_info['username']?>, can be here because is admin</div>
</body>
</html>
<?php
}
?>
\$\endgroup\$
1
  • \$\begingroup\$ I bet using your code the last line is not needed as it gives me error on line 27 <?php } ?> \$\endgroup\$ Apr 18, 2015 at 2:30
0
\$\begingroup\$

Security

You should use prepared statements when putting variable content into an SQL query, and you should encode special characters when echoing variable content.

It' hard to say if this is actually exploitable at the moment, but I could imagine various possible attacks, eg for SQL injection (a simple attack; depending on the allowed characters when registering, this might be a bit more complicated): a user registering under the name admin' # (assuming that the admin is named admin, this would not be a duplicate name, and it would result in this user gaining admin privileges). For XSS (this is a little far-fetched, but still): create user called admin' # <script src="evil.com/script.js"></script>, then - if there is no CSRF protection on the login - log the victim in as that user and make them visit that page.

Misc

  • don't have empty if blocks. If you don't need the if case, check the opposite, eg if ($user_level != 'ADMIN') { header(...); exit(); }
  • if you only use a variable in one location, you might want to remove it. Eg $user_level isn't really needed, just check if ($privileges["privs"] == 'ADMIN') directly. Same goes for $identification
\$\endgroup\$

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Not the answer you're looking for? Browse other questions tagged or ask your own question.