-1


My question is about my code. I have created 2 websites. But it's on mysqli PHP OOP. Someone told me that these queries are SQL Injection. So, i wanna change my database structure to prepare statements. Then, i can save my websites from SQL Injection. For that, I need your help. Here is my database structure:
config.php

<?php

define("DB_HOST", "localhost");
define("DB_USER", "root");
define("DB_PASS", "");
define("DB_NAME", "lunch");
?>

Database.php

<?php
    $filepath = realpath(dirname(__FILE__));
    include_once ($filepath.'/../config/config.php');
?>
<?php

Class Database {

    public $host = DB_HOST;
    public $user = DB_USER;
    public $pass = DB_PASS;
    public $dbname = DB_NAME;
    public $link;
    public $error;

    public function __construct() {
        $this->connectDB();
    }

    private function connectDB() {
        $this->link = new mysqli($this->host, $this->user, $this->pass, $this->dbname);
        if (!$this->link) {
            $this->error = "Connection fail" . $this->link->connect_error;
            return false;
        }
    }

    public function select($query) {
        $result = $this->link->query($query) or
                die($this->link->error . __LINE__);
        if ($result->num_rows > 0) {
            return $result;
        } else {
            return false;
        }
    }

    public function insert($query) {
        $insert_row = $this->link->query($query) or
                die($this->link->error . __LINE__);
        if ($insert_row) {
            return $insert_row;
        } else {
            return false;
        }
    }

    public function update($query) {
        $update_row = $this->link->query($query) or
                die($this->link->error . __LINE__);
        if ($update_row) {
            return $update_row;
        } else {
            return false;
        }
    }

    public function delete($query) {
        $delete_row = $this->link->query($query) or
                die($this->link->error . __LINE__);
        if ($delete_row) {
            return $delete_row;
        } else {
            return false;
        }
    }

}

?>

And i am writing my queries like this:

<?php
    $filepath = realpath(dirname(__FILE__));
    include_once ($filepath.'/../lib/Database.php');
?>

$name = mysqli_real_escape_string($this->db->link, $data['name']);
$query = "INSERT INTO users(name) VALUES('$name')";
$result = $this->db->insert($query);
if($result != false){
    header("Location: index.php");
}

$query = "SELECT * FROM users WHERE user_id = '$user_id'";
$result = $this->db->select($query);
$value = $result->fetch_assoc();
$name = $value['name'];

$query = "DELETE FROM users WHERE user_id = '$dlt_user'";
$result = $this->db->delete($query);
if($result){
    header("location: rd-user.php");
}

$query = "UPDATE users SET name = '$name' WHERE user_id = '$userid'";
$result = $this->db->update($query);
if ($result){
    header("Location: index.php");
}

Hey Your_Common_Sense brother! See this screenshot: Click to see screenshot
So, kindly please check my database structure and queries and tell me how can i change my db structure to prepare statements and how can i prevent from SQL Injections. Please help me.

1 Answers1

1

I find different methods in your class quite redundant. they all do the same thing, while differ only in the name. So first of all let's remove all but one method. I would also remove some other cargo cult code from the class as well.

Then you will need to rewrite your connection code in order to add the error reporting and the character set support. Add define('DB_CHARSET', 'utf8mb4'); to your config file as well.

Then you have to rewrite the only method left in this class in order to add the support of prepared statements. Refer to my article, Mysqli helper function for the details.

Class Database {

    public $link;

    public function __construct() {
        mysqli_report(MYSQLI_REPORT_ERROR | MYSQLI_REPORT_STRICT);
        try {
            $this->link = new mysqli(DB_HOST, DB_USER, DB_PASS, DB_NAME);
        mysqli_set_charset($this->link, DB_CHARSET);
        } catch (\mysqli_sql_exception $e) {
            throw new \mysqli_sql_exception($e->getMessage(), $e->getCode());
        }
    }

    public function query($sql, $params = [], $types = "")
    {
        if ($params) {
            $types = $types ?: str_repeat("s", count($params));
            $stmt = $this->link->prepare($sql);
            $stmt->bind_param($types, ...$params);
            $stmt->execute();
            return $stmt->get_result();
        } else {
            return $this->link->query($sql);
        }
    }
}

Now you can use your class with any kind of query utilizing prepared statements

$query = "INSERT INTO users(name) VALUES(?)";
$this->db->query($query, [$data['name']]);
header("Location: index.php");

$query = "SELECT * FROM users WHERE user_id = ?";
$result = $this->db->query($query, [$user_id]);
$value = $result->fetch_assoc();
$name = $value['name'];

$this->db->query("DELETE FROM users WHERE user_id = ?",[$dlt_user]);
header("location: rd-user.php");

$query = "UPDATE users SET name = ? WHERE user_id = ?";
$this->db->query($query,[$name,$userid]);
header("Location: index.php");
Your Common Sense
  • 156,878
  • 40
  • 214
  • 345
  • Can we use mysqli_real_escape_string() in this? to create variable like this: `$name = mysqli_real_escape_string($this->db->link, $data['name']);` or if we use prepare statements then we need to use simply like this: `$name = $data['name'];` ? – Zain Shabir Sep 23 '19 at 12:27
  • Brother, your code giving me a lot of errors. Please check the screenshot I've updated my question with screenshot of errors. – Zain Shabir Sep 23 '19 at 12:40
  • Zain, copying the code without understanding how it works is a recipe for disaster. In this case YCS made only a small typo, which would be easily fixable if you tried to understand the code. Just replace all occurrences of `$mysqli` with `$this->link` – Dharman Sep 23 '19 at 12:56
  • worked, but what about if i am not using `WHERE` condition in `SELECT` query, coz query($sql $param) requires 2 parameters, but if i want all data except `WHERE` then we don't need $params in query(), what i can do then? – Zain Shabir Sep 23 '19 at 13:36