Passing around Database Connection

DoubleDee · May 6, 2012, 7:36pm

I am concerned that passing my DB Connection string from file to file is a security risk…

The actual DB Connection string is located in mysqli_connect.php…


	// Make the connection.
	$dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
				OR die('Could not connect to database.  Contact System Administrator.');

Here is where I fear that I am doing something insecure…

In profile.php script, I include this DB connection like this…


		// Connect to the database.
		require_once(WEB_ROOT . 'private/mysqli_connect.php');

And then farther down in that same script I call this function…


			// Get # of Posts.
			$numberOfPosts = getNumberOfPosts($dbc, $id);

The problem - as I found out last night - is that I can’t require the DB Connection from inside my Function, so it looks like I have to pass it as an argument to my Function as seen above.

I am concerned that I am passing my Database’s Username and Password for all to see in plain sight by doing this?!

I need some serious help/advice here!!

Debbie

chris_upjohn · May 6, 2012, 11:39pm

No there are no security risks involved when passing in a database connection using a file include/require, as long as your file permissions for ALL php files are set to 0644 on Apache servers or 0666 on CGI servers you should never have to worry about someone been able to access your database information. As for passing in the database connection itself the easiest way is to just use the global keyword inside your function like the below example.

function getNumberOfPosts($id) {
    global $dbc;
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

DoubleDee · May 7, 2012, 12:04am

chris_upjohn:

No there are no security risks involved when passing in a database connection using a file include/require, as long as your file permissions for ALL php files are set to 0644 on Apache servers or 0666 on CGI servers you should never have to worry about someone been able to access your database information. As for passing in the database connection itself the easiest way is to just use the global keyword inside your function like the below example.
function getNumberOfPosts($id) {
    global $dbc;
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

Using global anything is a bad idea, so I’d have to agree with that suggestion.

Debbie

oddz · May 7, 2012, 1:50am

Given your working with procedural code and *should avoid global variables I would recommend using statics.


function getDb($conn=null) { /* probably a bad name but I never do this and have no idea what to call it... I think drupal uses get for the same concept without OO */
	static $db;

	if($conn !== null) {
		$db = $conn;
	}
	
	return $db;
}

Upon setting up the connection call the function passing the db resource and when you would like to get it after call the function without passing an arguments:


    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
                OR die('Could not connect to database.  Contact System Administrator.');  

   // set the connection reference
   getDb($dbh);


function getNumberOfPosts($id) {

    $dbc = getDb(); /* now the function has a dependency on getDb() but I believe this is good method given the procedural nature of the code */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

In my opinion that is still really dirty and undesirable but it is probably the best solution aside from creating an object-oriented data access layer which would require completely changing existing architecture.

The other solution is passing the connection as an argument to every function that is dependent on it.


function getNumberOfPosts($id,$dbc) { /* all data access function would require passing the connection... using a function seems better */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

At least by using a function to get and set the connection some code repetition can be eliminated.

DoubleDee · May 7, 2012, 2:06am

I’m sorry, what are you saying that I should do?? (I could use a little hand-holding on this one…)

BTW, someone told me having my DB Connection string located in mysqli_connect.php and using constants like this was a bad idea…


<?php
	// This file contains the database access information.
	// It also establishes a connection to MySQL and selects the database.

	// Set the database access information as constants.
	DEFINE('DB_ENVIRONMENT', 'development');
//	define('DB_ENVIRONMENT', 'production');

	// Database Host.
	DEFINE('DB_HOST', ENVIRONMENT === 'development'
					? 'localhost'
					: '');

	// Database User.
	DEFINE('DB_USER', ENVIRONMENT === 'development'
					? 'root'
					: '');

	// Database Password.
	DEFINE('DB_PASSWORD', ENVIRONMENT === 'development'
					? 'root'
					: '');

	// Database Name.
	define('DB_NAME', ENVIRONMENT === 'development'
					? 'doubledee'
					: '');

	// Make the connection.
	$dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
			OR die('Could not connect to database.  Contact System Administrator.');

They said using Constants was a dumb idea…

Debbie

oddz · May 7, 2012, 2:10am

I updated my response with more detail.

As for db configuration. Most systems store that information in some type of configuration file. Any configuration that changes based on environment should be stored in some type of configuration file and there should be a system in place to manage that info. Hard coding things may work but it is not ideal. Though the more ideal things become the more complex the code will become. So it is a trade off.

None the less, managing configuration is really another topic.

DoubleDee · May 7, 2012, 2:32am

oddz:

Given your working with procedural code and *should avoid global variables I would recommend using statics.
function getDb($conn=null) { /* probably a bad name but I never do this and have no idea what to call it... I think drupal uses get for the same concept without OO */
	static $db;

	if($conn !== null) {
		$db = $conn;
	}
	
	return $db;
}
Upon setting up the connection call the function passing the db resource and when you would like to get it after call the function without passing an arguments:
    // Make the connection.
    $dbc = @mysqli_connect(DB_HOST, DB_USER, DB_PASSWORD, DB_NAME)
                OR die('Could not connect to database.  Contact System Administrator.');  

   // set the connection reference
   getDb($dbh);

I have LOTS of questions…

1.) This other person I was talking to made it sound like I should create DB Connection and then delete the components (e.g. DB name, DB password, etc) that created that DB Connection?! :-/

What do you think?

2.) Can the DB Connection be “reverse engineered” so that a hacker could get the DB name, DB password, etc?

3.) According to the PHP Manual…

A static variable exists only in a local function scope, but it does not lose its value when program execution leaves this scope.

Doesn’t that mean that $db would cease to exist out of the getDb function?

4.) What is wrong with how I am doing things now?


function getNumberOfPosts($id) {

    $dbc = getDb(); /* now the function has a dependency on getDb() but I believe this is good method given the procedural nature of the code */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

5.) Based on an error in my earlier code, I am pretty sure that a Function cannot see outside of itself. So how can you call your function from within the getNumberOfPosts fucntion?

6.) Why can’t you just create another parameter and pass $db?

7.) And again, why is it bad to pass any Database Connection? (I’m not understanding were the “Attack Vectors” exist…

In my opinion that is still really dirty and undesirable but it is probably the best solution aside from creating an object-oriented data access layer which would require completely changing existing architecture.

I don’t understand why you don’t like this… :scratch:

The other solution is passing the connection as an argument to every function that is dependent on it.


function getNumberOfPosts($id,$dbc) { /* all data access function would require passing the connection... using a function seems better */
    
    if (!$res = $dbc->query('SELECT COUNT(id) AS total FROM posts WHERE id = ' . $id)) {
        die('MySQL Error: ' . $res->error);
    }
    
    return $res->num_rows;
}

At least by using a function to get and set the connection some code repetition can be eliminated.

Hmm…

Debbie

oddz · May 7, 2012, 2:52am

You could but I don’t see a benefit unless the program allows input of php from the UI and evals it out. Though at that point your looking at a whole slew of potential security issues. i don’t think this is the case with what your doing though so it isn’t really worth getting into indepth.

Not unless you do the above allowing people to enter PHP from a text area and eval it out. Ideally the db authentication info itself should be stored outside the site root to prevent any type of access beyond someone hacking the server itself.

Correct. If it existed outside the function and was accessible in any function it would be a global variable. The whole idea of a static variable is that the value can be managed from a single point, the function which it is defined. It is a tad less evil than a global though still unideal. When you start talking about storing state the best option is to move to an OO approach but that becomes much more complex. So what I presented I believe to be the best solution given the existing architecture, code flow and learning curve.

Functions can call other functions as long as they exist. That is a none issue. It only becomes more complex when introducing OOP (methods) into the mix.

You can but my preference is a getter/setter function to eliminate passing the db connection all the time.

It isn’t necessarily bad. No security issues exist with that. Like I said, I was using a getter/setter function to eliminate function arguments and avoid passing the connection around.

It just all seems really dirty considering I’m more an MVC, OOP guy. You have to start somewhere though and I can’t say I have never written nor managed similar code because I have. Though a lot of things change in 3 years. If this is what your comfortable with than so be it. I was merely providing a disclaimer for someone who comes along and starts to mention a more advanced method to handle things, like a OO data access layer. If someone else wants to get into a whole conversation over using OO to do this they can but my recommendation was provided with the smallest leanring curve in mind, avoiding global variables.

DoubleDee · May 7, 2012, 3:32am

oddz,

Thanks for the detailed responses so far!! :tup: