Pulling Data from database using PDO

PHP
1

Dear All,
I am trying to pull data from the database using PDO, is the code below safe from an attack or do I need to bind anything? If I need to bind anything, please help me write the correct code because all the videos that I bought on e-commerce used mysqli and it is the procedural way without validating anything in the name of keeping things simple.

    function productsInProductCategories() { 
    global $connection;   
   
    if(isset($_GET['p_cat'])){

        $p_cat_id = $_GET['p_cat'];

        $get_p_cat = "SELECT * FROM product_categories WHERE p_cat_id='$p_cat_id'";

        $run_p_cat = $connection->query($get_p_cat);

        $row_p_cat = $run_p_cat->fetch();

        $p_cat_title = $row_p_cat['p_cat_title'];

        $p_cat_desc = $row_p_cat['p_cat_desc'];

        $get_products = "SELECT * FROM products WHERE p_cat_id='$p_cat_id'";

        $run_products = $connection->query($get_products);

        $count = $run_products->rowCount();

        if($count==0){

            echo "

            <div class='box'>

            <h1> No Product Found In This Product Category </h1>

            </div>

            ";

        }else{

            echo "

            <div class='box'>

            <h1>$p_cat_title</h1>

            <p>$p_cat_desc</p>

            </div>

            ";

        }

        while($row_products = $run_products->fetch()){

            $pro_id = $row_products['product_id'];

            $pro_title = $row_products['product_title'];

            $pro_price = $row_products['product_price'];

            $pro_img1 = $row_products['product_img1'];

            echo "
                <div class='col-md-4 col-sm-6 single'>
                    <div class='product'>
                        <a href='details.php?pro_id=$pro_id'>
                        <img
                            src='admin_area/product_images/$pro_img1'
                            class='img-fluid'
                        />
                        </a>

                        <div class='text'>
                        <h3><a href='details.php?pro_id=$pro_id'>$pro_title</a></h3>

                        <p class='price'>$$pro_price</p>

                        <p class='buttons'>
                            <a href='details.php?pro_id=$pro_id' class='btn btn-outline-primary'>View details</a>

                            <a href='details.php?pro_id=$pro_id' class='btn btn-primary'>
                            <i class='fa fa-shopping-cart'></i> Add to cart
                            </a>
                        </p>
                        </div>
                    </div>
                </div>
            
            ";

        }


    }
}

Please, help me with the code that will work instead of just suggesting what to do, it will help me write the code faster and also make it more clearer.

2

First thing is you need to be using prepared statements.

1 Like
3

This is often quoted as a good source for learning PDO :

3 Likes
4

Thanks for the link, I know how to insert data with prepare statement in PDO but what I don’t rely know is how to pull data from database using prepare statement, if anyone can help with the code above it will make it more clearer to me and and time safer, then I will continue from there in my project.

5

No, it’s not safe at all.
Here is the problem:-

The data from $_GET can be anything at all the user wants to put in it. It’s just a case of them writing the URL with a harmful value to the p_cat query string.

www.example.com?p_cat=some-nasty-sql-injection

And you submit that query to your database…

It’s safer to use a prefpared statement:-

        $p_cat_id = intval($_GET['p_cat']); // ensures the value in a integer

        $get_p_cat = "SELECT * FROM product_categories WHERE p_cat_id = ?"; // Use a ? placeholder for the variable value

        $run_p_cat = $connection->prepare($get_p_cat); // Sends the statement to the database

        $run_p_cat->execute([$p_cat_id]); // Execute the query, passing in the value

        $row_p_cat = $run_p_cat->fetch(); // Fetch the data

How does that help?
An injection attack can work by modifying the statement you make, possibly by ending it, then starting a whole new statement, all within the value passed in.
But when you prepare a statement, the DB knows what the whole structure of the query will look like from start to finish, excluding the user submitted values. So it’s all there hard-coded, free of tampering. In this case it’s a single select query. There is no second query that drops a table or fetches passwords from the user table, it just does what you wanted it to and nothing more.
Only after then, you pass in the user data. Because the structure of the whole statement is already defined, that user data will only ever be considered as a value to search for, it can’t be a termination of the initial query, followed by a second spurious query, because that’s not how the pre-defined, hard-coded, untampered statement was.

1 Like
6

I have read the forwarded tutorials and they really helped me, but I have another question about LIMIT Clause, is posted under my reply to SamA74 below, help check if I am right.

7

Thanks, your explanation helped me, I have gotten the answer but there is one more which use LIMIT clause, here is the code if I’m right :

function getProPDO() {
  
global $pdo;

$recordStart = 0;
$productSize = 8;
//$productSize = intval($productSize);

$sql = $pdo->prepare("SELECT * FROM products ORDER BY 1 DESC LIMIT ?, ?");

$sql->bindValue(1, $recordStart,  PDO::PARAM_INT );
$sql->bindValue(2, $productSize,  PDO::PARAM_INT);

$sql->execute();

$result =  $sql->rowCount();

if($result === 0) exit('No rows');

while($row= $sql->fetch()) {
    $pro_id = $row['product_id'];

    $pro_title = $row['product_title'];

    $pro_price = $row['product_price'];

    $pro_img1 = $row['product_img1'];

    echo "
        <div class='col-md-4 col-sm-6 single'>
            <div class='product'>
                <a href='details.php?pro_id=$pro_id'>
                <img
                    src='admin_area/product_images/$pro_img1'
                    class='img-fluid'
                />
                </a>

                <div class='text'>
                <h3><a href='details.php?pro_id=$pro_id'>$pro_title</a></h3>

                <p class='price'>$$pro_price</p>

                <p class='buttons'>
                    <a href='details.php?pro_id=$pro_id' class='btn btn-outline-primary'>View details</a>

                    <a href='details.php?pro_id=$pro_id' class='btn btn-primary'>
                    <i class='fa fa-shopping-cart'></i> Add to cart
                    </a>
                </p>
                </div>
            </div>
        </div>
    
    ";
}

}

8

As the values for limit are integers you do not need to use prepared statements here. You can just use intval(). It’s also safe

1 Like
9

Thanks.

10

As these values are hard coded by yourself, there is no need to use placeholders, or to prepare and execute. You may simply put the value directly into the statement.

1 Like
11

Please, pardon me one more thing, is this one also safe?

$sql = $pdo->prepare("SELECT * FROM product_categories");

        $sql->execute();

        while($row= $sql->fetch()) {
            $p_cat_id = $row['p_cat_id'];

            $p_cat_title = $row['p_cat_title'];

            echo "<li> <a href='shop_pdo.php?p_cat=$p_cat_id' class='nav-link'> $p_cat_title </a> </li>";
        }
12

This depends on what data is in the table product_categories.
Is it possible for the user to modify data in this table or insert new data?
Do you validate the data which is stored into this table?

If not, you need to use htmlentities() to make the string secure

1 Like
13

Because you are not suppling a value to this query, there’s no point to prepare it first. Just use the ->query() method.

1 Like
14

The risk is not the reading from database but the creating of html from the database content. It’s very easy to put an XSS attack in a database string if you do not take care of that

1 Like
15

I validated the data that was stored in the database.

16

As you only use two columns here, p_cat_id and p_cat_title, you should really only retrieve those columns in your query:

select p_cat_id, p_cat_title from product_categories

I also don’t see why you’ve created the variables $p_cat_id and $p_cat_title - why not echo your $row elements directly?

3 Likes
17

Thanks for shedding more light to the query.

18

Sorry folks but the LIMIT advice which involves directly adding values into the sql string is very very very bad. The whole point behind prepared statements is to avoid the need for the programmer to take specific action based on data types. If you are going to the trouble of using prepared statements then all variables should be bound regardless of source or types.

In the original post the bindParam should have been used instead of bindValue.

19

The mantra is: validate on input, escape on output.
So you would typically do both these things.

I think the point there was more that prepare and execute is redundant code in this case.
You are correct about the XSS risk.

I was thinking along the lines of:-

$sql = $pdo->query("SELECT * FROM products ORDER BY 1 DESC LIMIT 0, 8");

Anything wrong with that?

I would just put an array into execute(), I have never used bind anything.

20

It is generally recommended not to SELECT * but to select the actual fields you want to use.