HiveBrain v1.2.0
Get Started
← Back to all entries
patternphpModerate

Database Handler Class

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
databasehandlerclass

Problem

I've written this DB Handler class. Please review it and suggest any code edits or point out mistakes and security loop holes. Please also suggest a better way to handle things, if there is a security loop hole in there.

``
class db {

protected $dbHost;
protected $dbUser;
protected $dbPass;
protected $dbName;
protected $dbLink;

public function __construct ( $host=NULL, $user=NULL, $pass=NULL, $db=NULL ){

$this->dbHost = $host? $host : 'localhost';
$this->dbUser = $user? $user : 'root';
$this->dbPass = $pass? $pass : '';
$this->dbName = $db? $db : 'my_db_name';

if( !($this->dbLink = mysqli_connect( $this->dbHost, $this->dbUser, $this->dbPass, $this->dbName) ) ){
echo 'Error While Connection to DB: '.mysqli_error( $this->dbLink );
}
}

public function __destruct(){
if( ! mysqli_close($this->dbLink) ){
echo 'Error While Closing Connection to DB: '.mysqli_error( $this->dbLink );
}
}

/

1) This function is used to fetch data

@param: $tableName [required]
@param: $requiredColumns. This is an array like 'tableColumn', 'tableColumn', 'tableColumn'
@param: $conditionsArray. This is an associative array like 'tableColumn' => 'value to check'
@param: $orderBy. This is an array like 'tableColumn', 'ASC'

*/
public function select( $tableName, $requiredColumns = NULL, $conditionsArray = NULL, $orderBy = NULL ){
if( $requiredColumns == NULL || count($requiredColumns) $value ){
array_push( $args , '
'.$key.'` = "'.$value.'"' );
}
//var_dump( $args );
$where = implode( ' AND ' , $args );
$query .= ' WHERE ' . $where;
}
if( $orderBy != NULL ){
$order = implode(' ', $orderBy);
$query .= 'ORDER BY '.$order;
}
//echo $query;
return $this->execute( $query , 'resource' );

Solution

Security

This code is not secure.

SQL Injection

You are trusting the user input completely (except in the login function where you do use mysqli_real_escape_string, which is not secure enough). Use prepared statements instead. You should also read up on SQL injections in general.

Other

This is all minor in comparison to the SQL injection.

  • don't hardcode your password in the PHP code, store it in an external configuration file (outside the web root as otherwise your passwords will be exposed).



  • don't echo complete errors to the user. Use a custom error string instead.



  • md5 is not a good enough hashing function, use something like bcrypt instead. (see for example here and here)



Code

query function

The query function just renames the execute function, and is thus useless. Also, you are not even using it. I would just remove it (and make execute public if you need to).

I think that you should fix the SQL injection issue before the rest of the code is reviewed, as it will change in a lot of places.

Context

StackExchange Code Review Q#61231, answer score: 11

Revisions (0)

No revisions yet.