Jump to content

Recommended Posts

I am creating a class to control my database access, and I am wondering what people think the security risks of the following code to be.

<?php
    function DBManager($host, $user, $pass, $db){
        $startTime = $this->getMicroTime();
        
        // Try to make a connection to the server
        if (!$this->connection = @mysql_connect($host,$user,$pass,true)){
            $this->errorCode = mysql_errno();
            $this->errorMsg  = mysql_error();
            return false;
        }
        
        // Now select the database
        if (!@mysql_select_db($db,$this->connection)){
            $this->errorCode = mysql_errno();
            $this->errorMsg  = mysql_error();
            @mysql_close($this->connection);
            return false;
        }

        $this->totalTime += $this->getMicroTime() - $startTime;
            
        return true;
    }
?> 

 

Usually I store my database connection script outside my public_html folder, but I will not be able to do so in this case.

 

Does anyone have any thoughts or perhaps a better way of coding this?

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/
Share on other sites

It is a very good idea to have a Class that provides an abstraction layer for your database functions.

 

I always use one when working on any dynamic site, here are the two main pro's:

1. It provides all the commonly used database functions you would need

2. It is generic, for MS SQL or other databases, just change the insides of the functions, and you have abstraction for easy reuse

 

If you have one for MySQL, PostGre, SQLite, MS SQL etc, your application is only working with the 'Database' class, and it cares not what goes on under the hood of it. It makes it super easy to port apps to other databases.

 

Here is the idea of what I use.

 

"//@" explains my code.

 

<?php


//=======MySQL Connection details============

//@ these can be kept in a config file, especially if there is plenty of other 'config' data
$host = "localhost";
$database = "dbname";
$user = "username";
$password = "password";

//@ keep your different settings at hand while in development - it saves me some time.
/* 
====Local Settings
$host = "localhost";
$database = "dbname";
$user = "username";
$password = "password";

====development server settings
$host = "localhost";
$database = "dbname";
$user = "username";
$password = "password";

====production server settings
$host = "localhost";
$database = "dbname";
$user = "username";
$password = "password";

*/
//=====================


//Database Constants
defined('DB_HOST') 		? null : define("DB_HOST", $host);
defined('DB_NAME') 		? null : define("DB_NAME", $database);
defined('DB_USER') 		? null : define("DB_USER", $user);
defined('DB_PASS') 		? null : define("DB_PASS", $password);


class MySQLDatabase{

	//attributes
	private $connection;
	public $last_query;
	private $magic_quotes_active;
	private $real_escape_string_exists;

public function __construct(){

	//@ open the connection at construction, so it is available from the start
	$this->open_connection();
	$this->magic_quotes_active = get_magic_quotes_gpc();
	$this->real_escape_string_exists = function_exists( "mysql_real_escape_string" );
}

public function open_connection(){
	$this->connection = mysql_connect(DB_HOST, DB_USER, DB_PASS); //open connection
	if(!$this->connection){
	die("Error opening MySQL Database connection: ". mysql_error());
	}else{
		$db_select = mysql_select_db(DB_NAME, $this->connection); //select database
		if (!$db_select){
			die("Database selection failed: " . mysql_error());
		}
	}
}

public function close_connection(){
		if(isset($this->connection)){
	mysql_close($this->connection);
	unset($this->connection);
	}
}

//@ to do a simple query, and return the resultset
public function query($sql){

	$this->last_query = $sql; //set last query
	$result = mysql_query($sql, $this->connection);
	  
		if (!$result) { //check for error

			//output the last query - usefull for debugging
			$output = "Last Query ".$this->last_query."<br>";
			$output .= "Database query failed: " . mysql_error() . "<br /><br />";
			die( $output );
		}		
	return $result;		
}

//@ this is some rather tricky, it just returns the escaped paramater
public function escape_value($value) {

	if($this->real_escape_string_exists) { 
		// undo any magic quote effects so mysql_real_escape_string can do the work. PHP v4.3.0 or higher
		if( $this->magic_quotes_active ){
		$value = stripslashes( $value ); 
		}
		$value = mysql_real_escape_string($value);
	} 
	else{
		// if magic quotes aren't already on then add slashes manually, before PHP v4.3.0
		if( !$this->magic_quotes_active ){
			$value = addslashes( $value );
		}
		// if magic quotes are active, then the slashes already exist
	}
	return $value;
}

//database neutral methods
public function fetch_array($result){
	 return mysql_fetch_array($result);
  	}

public function num_rows($result){
	return mysql_num_rows($result);
}

public function get_last_id(){
	return mysql_insert_id($this->connection); // get the last id inserted over the current db connection
}

public function affected_rows(){
	return mysql_affected_rows($this->connection);
}

public function insert_id() {
    // get the last id inserted over the current db connection
    return mysql_insert_id($this->connection);
  }

}

//@ make the $database and $db variables available for use.
$database = new MySQLDatabase();
$db =& $database;
?>

 

This approach becomes really effective when using an OOP design, in which you have clas modelling each of your data objects, which each in turn has a table. You can build the models inteligently so that they 'know how to' construct, update and create themselves (in and from the their tables in the DB) by making use of the database class.

 

It works for me!

 

Regards

 

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/#findComment-911627
Share on other sites

Oh I got carried away... security. Just make sure all your data going into your database are escaped. If you use 'model' classes for your data objects (eg users, cars, photos, whatever) knowing how to update/create themselves using, for example, $user->save(); or $user->create(); , escape all the attributes in the create/save functions prior to insertion.

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/#findComment-911630
Share on other sites

1. It provides all the commonly used database functions you would need

 

So does PHP's standard library.

 

2. It is generic, for MS SQL or other databases, just change the insides of the functions, and you have abstraction for easy reuse

 

That's somewhat of a pipe dream. Migrating to another database system is unlikely to be the trivial task you make it sound like. That is unless you restrict yourself to the very basic UPDATE/DELETE/INSERT statements of course.

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/#findComment-911655
Share on other sites

mmm... pipe dream indeed - just burst my bubble  :'( I should have said:

 

1. It groups all of the commonly used db functions you commonly use together in one place

 

2. It is reasonably generic, and provides a bit of abstraction between databases as far as simple functionality is concerned

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/#findComment-911662
Share on other sites

Hate to seem like we're picking on you, but I mean this advice in a helpful way.

 

 

Instead of using constants to store your DB config, it would be better idea to pass the config to the database constructor.  That way the database config is in the scope of the database connection, and the better reason is that then you could have multiple instances with unique configurations.

Link to comment
https://forums.phpfreaks.com/topic/172221-database-security/#findComment-912062
Share on other sites

This thread is more than a year old. Please don't revive it unless you have something important to add.

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Restore formatting

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...

Important Information

We have placed cookies on your device to help make this website better. You can adjust your cookie settings, otherwise we'll assume you're okay to continue.