Jump to content

Security Issue On Using Globals


Redroest

Recommended Posts

I am building a new OOP based CMS system with modules etc. Now I am concerned about using globals in my functions. My system uses a mysqli class to connect with the database and returns the connection in a variable $mysqli:

$db = array (  'host' => $db_host,  'user' => $db_user,  'pass' => $db_pass,  'dbname' => $db_name);$mysqli = new mysqli($db['host'], $db['user'], $db['pass'], $db['dbname']);if(mysqli_connect_errno()){  trigger_error('Fout bij verbinding: '.$mysqli->error);}

When I try to call a module like news my url will be: index.php?module=news&page=NewsListThis triggers the NewsList() function inside the module 'news' using the code below.

switch ($page) {	case 'NewsList':		NewsList();	break;	default:		index();	break;}function NewsList(){  global $mysqli;  include_once('header.php');  OpenDiv();  $sql = "SELECT * FROM newstable";  while($news= $mysqli->query($sql))  {	echo $news['title'].'<br />';  }  CloseDiv();  include_once('footer.php');}

As you can see I am using global $mysqli to pass the connection into the function. But in articles on the web they warn me about the security issues concerning globals and $GLOBAL. One method to make $mysqli act like a superglobal is to call the database connection into a function and then return it like:

function db(){  $db = array (	'host' => $db_host,	'user' => $db_user,	'pass' => $db_pass,	'dbname' => $db_name  );  $mysqli = new mysqli($db['host'], $db['user'], $db['pass'], $db['dbname']);  if(mysqli_connect_errno())  {	trigger_error('Fout bij verbinding: '.$mysqli->error);  }  return $mysqli;}//and when calling the connection use:function NewsList(){  include_once('header.php');  OpenDiv();  $mysqli = db();  $sql = "SELECT * FROM newstable";  while($news= $mysqli->query($sql))  {	echo $news['title'].'<br />';  }  CloseDiv();  include_once('footer.php');}

That last type of passing the connection looks safer to me because I am not using any globals, Downside is that the NewsList function relies on the fact that there must be a db() function in order to let the NewsList function to work. Does anyone of you know the best way to deal with these issues?

Link to comment
Share on other sites

It also needs a global variable, so either way it needs something from the outside. The major downside with the second part is that it requires a different connection every time you go to the database. If you used a persistent connection that would help that part.But why do you think global variables are bad? Are they always bad, or are there situations where it's fine to use them? If global variables are always bad to use, why are they an option in so many languages? What security issues do the articles warn you about, and do they apply to your situation?

Link to comment
Share on other sites

Maybe he was thinking about register_globals?Anyway, as PHP is a fundamentally procedural language with no integral namespace functionality, you can't avoid declaring things that are global anyway (e.g. your functions). Of course, passing data from function to function by the use of global variables isn't exactly best practice, but without resorting to the encapsulation possibilities of object-oriented code this is hard to rectify in some cases.

class Database {	private $mysqli;	public __construct($host, $user, $pass, $db); // initialize $mysqli	public showPage($name);}

//...$db = new Database($db_host, $db_user, $db_pass, $db_name);$db->showPage($page);//...

Addendum: note that only class names are supposed to be named in CamelCase.

Link to comment
Share on other sites

That article doesn't really mention security at all, his arguments against using global variables are more about maintaining and extending the code and issues that arise from using globals. He presents a couple use cases for globals, why they make either maintaining or extending the code difficult, and what a better alternative is for that case.The purpose of all of my questions was just to get you to think. Don't memorize things like "global variables are bad", understand the issues around them and why people choose to use them, why they may not be the best choice, and what alternatives are available.There are no security issues with your first code, at least as far as globals are concerned. If the server you were running this on did in fact have register_globals enabled (but why would it?), then someone could overwrite the $mysqli variable with... what? Not another object, you can't pass an object instance through $_GET, $_POST, or $_COOKIE. You could overwrite it with a scalar value, then what? The script will get a fatal error when it tries to reference a member of the object and the script will end. That's it, that's the worst case. The problem with using your global object has nothing to do with security, it's an issue of maintaining and extending the code.One solution would be to pass the database object as a parameter to your function.

Link to comment
Share on other sites

Thanks.. you are the best :)I have now something like this in my configuration file:

<?php//Configuration globals$db_host = 'localhost';$db_user = 'xxxx';$db_pass = 'xxxx';$db_name = 'xxxx';$defaultTheme = 'Redroest';$defaultLang = 'NL';//all other globals like abovefunction GetTheme(){ //Query with the users chosen theme as output into $theme;  if(!isset($theme))  {	return $GLOBAL['defaultTheme '];  }  else  {	return $theme;  }}?>

Link to comment
Share on other sites

http://en.wikipedia.org/wiki/Encapsulation...)#Encapsulation and http://en.wikipedia.org/wiki/Encapsulation...ed_programming)Basically, you declare the $mysqli variable as a member of the class that contains all your database functions - then it's accessible to them, but not available for just any script to mess around with.
Link to comment
Share on other sites

Archived

This topic is now archived and is closed to further replies.

×
×
  • Create New...