Safeguard mysql CRUD

Discussions of secure PHP coding. Security in software is important, so don't be afraid to ask. And when answering: be anal. Nitpick. No security vulnerability is too small.

Moderator: General Moderators

Post Reply
markthien
Forum Commoner
Posts: 33
Joined: Fri Feb 13, 2009 7:50 pm

Safeguard mysql CRUD

Post by markthien »

Hi guys,

I am connecting to mysql database like below:

db-connection.php

Code: Select all

<?php

class DBConnection {
	
	private $con = null;
	private $host = 'localhost';
	private $connection_string = 'mysql:host=localhost;dbname=ifnoresponse';
	private $username = 'ifnoresponse';
	private $dbname = 'ifnoresponse';
	private $password = 'ifnoresp*pass';
	
	function __construct() {
	}
	
	public function getConnection(){
		if($this->con == null){
			try {
				$this->con = new PDO($this->connection_string, $this->username, $this->password);
				$this->con->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
			} catch(PDOException $e) {
				error_log($e->getMessage());
			}			
		}
		return $this->con;
	}
	
	function closeConnection(){
		$this->con = null;
	}
	
	public function getHost(){
		return $this->host;
	}
	
	public function getUsername(){
		return $this->username;
	}

	public function getPassword(){
		return $this->password;
	}	
	
	public function getDbname(){
		return $this->dbname;
	}		
}

?>
get-user.php

Code: Select all

<?php
	
header('Content-type: text/html; charset=UTF-8');
header('Cache-Control: no-cache');
require_once('db-connection.php');

$conn = new DBConnection();
		try{
		
			// the actual query for the grid data 
			$sql = "select count(*) as cnt from user where username = :username";
			
			$stmt = $conn->getConnection()->prepare($sql);			
			$stmt->bindValue(':username', $email, PDO::PARAM_STR);
			
			$count = 0;
			
			$stmt->execute();	
			
			while ($row = $stmt->fetch()) {
				  $count = $row['cnt'];
			}
			
	               $stmt->closeCursor();
                       $conn->closeConnection();
		} catch(PDOException $e) {
			error_log($e->getMessage());
		}	

?>
Appreciate any advice please. Thanks in advance!

best regards,
Mark Thien
User avatar
getmizanur
Forum Commoner
Posts: 71
Joined: Sun Sep 06, 2009 12:28 pm

Re: Safeguard mysql CRUD

Post by getmizanur »

i'm not sure what you want here, are you worried about sql injection or to check if the class structure is right

code encapsulated in your dbconnection class look okay to me however i would declare all get functions as static and use public access specifier. this would avoid instantiating dbconnection every time you need it. so your code would look like this

db-connection.php

Code: Select all

<?php

class DBConnection {
        
        private $con = null;
        private $host = 'localhost';
        private $connection_string = 'mysql:host=localhost;dbname=ifnoresponse';
        private $username = 'ifnoresponse';
        private $dbname = 'ifnoresponse';
        private $password = 'ifnoresp*pass';
        
        function __construct() {
        }
        
        public static function getConnection(){
                if($this->con == null){
                        try {
                                $this->con = new PDO($this->connection_string, $this->username, $this->password);
                                $this->con->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
                        } catch(PDOException $e) {
                                error_log($e->getMessage());
                        }                       
                }
                return $this->con;
        }
        
        function static closeConnection(){
                $this->con = null;
        }
        
        public static function getHost(){
                return $this->host;
        }
        
        public static function getUsername(){
                return $this->username;
        }

        public static function getPassword(){
                return $this->password;
        }       
        
        public static function getDbname(){
                return $this->dbname;
        }               
}
get-user.php

Code: Select all

<?php
header('Content-type: text/html; charset=UTF-8');
header('Cache-Control: no-cache');
require_once('db-connection.php');

                try{
                
                        // the actual query for the grid data 
                        $sql = "select count(*) as cnt from user where username = :username";
                        
                        $stmt = DBConnection::getConnection()->prepare($sql);                  
                        $stmt->bindValue(':username', $email, PDO::PARAM_STR);
                        
                        $count = 0;
                        
                        $stmt->execute();       
                        
                        while ($row = $stmt->fetch()) {
                                  $count = $row['cnt'];
                        }
                        
                       $stmt->closeCursor();
                       DBConnection::closeConnection();
                } catch(PDOException $e) {
                        error_log($e->getMessage());
                }       
plus, all user input should be considered as tainted and you should use mysql_real_escape_string for those inputs such as username
Post Reply