KnottyAlder Posted August 28, 2009 Share Posted August 28, 2009 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? Quote Link to comment Share on other sites More sharing options...
Daniel0 Posted August 30, 2009 Share Posted August 30, 2009 How would it be a security risk in respect to the database? Quote Link to comment Share on other sites More sharing options...
mystery_man Posted September 3, 2009 Share Posted September 3, 2009 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 Quote Link to comment Share on other sites More sharing options...
mystery_man Posted September 3, 2009 Share Posted September 3, 2009 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. Quote Link to comment Share on other sites More sharing options...
Daniel0 Posted September 3, 2009 Share Posted September 3, 2009 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. Quote Link to comment Share on other sites More sharing options...
mystery_man Posted September 3, 2009 Share Posted September 3, 2009 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 Quote Link to comment Share on other sites More sharing options...
Daniel0 Posted September 3, 2009 Share Posted September 3, 2009 http://php.net/pdo Quote Link to comment Share on other sites More sharing options...
corbin Posted September 3, 2009 Share Posted September 3, 2009 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. Quote Link to comment Share on other sites More sharing options...
Recommended Posts
Join the conversation
You can post now and register later. If you have an account, sign in now to post with your account.