Selecting a table with multiple ID on MySQL using PHP PDO

Question

I have made a MySQL script to query a table with a set of id using an IN clause and after some reading I found out that there are security issues with it like prone to SQL injections and the likes.

Are there other ways to write this code with a more secure way? Although I'm using PDO, is that really enough?

 public function GetAllCustomerFilter($search, $cat_id)
 {    
   $stmt = $this->db->prepare("SELECT * from users_cards WHERE cat_id IN ($cat_id) AND username LIKE :search ORDER BY id DESC LIMIT 60";
   $stmt->bindParam(":search", $search);
     if($stmt->execute()) {
       $products =  $stmt->fetchAll(PDO::FETCH_ASSOC);
     }
 }

Data is received from an Ajax:

    loadProductsListPerCat = function(form,data_serve) {

    var data_fed = new Array();
    $.each(data_serve, function(i,obj)
    {
        data_fed[i] = obj.cat_id;
    });
    var raw = data_fed.join();
    var encoded = encodeURIComponent(raw);
    var formData =  "data_serve="+encoded+"&"+ $(form).serialize(); 
    var requestURL = 'searchUsers';
    console.log(formData);
    $.ajax({
        url: requestURL,
        data: formData,
        crossDomain: true,
        dataType: 'json',
        cached: false,
        async: true,
        success: function(data) {
            console.log(data);

        },
        complete: function() {
            if(isAppend) {
                $('.btn-toggle-load-more-users').attr('disabled', false);
            }
        },
        error: function(XMLHttpRequest, textStatus, errorThrown) {

        }
    });    
};

The variable data_serve is taken by another jQuery function on which it counts input elements that has data-on set to true:

var PCatVal = (function(){
    var cats = new Array();

    $("#main_cat").find(".option_").each(function(index) {
        $(this).removeClass("option_");
        $(this).addClass("ctrls option_"+index);

        $(".option_"+index).on("click", function(e){
            var opt = $(this).data('parentname');
            var opt_id = $(this).val();
            var data_serve = [];
            //array check if exists
            var idx = $.inArray(opt_id, cats);
                if (idx == -1) {
                   cats.push(opt_id);  
                } else {
                    cats.splice(idx, 1);
                }
           //loop the array so to encode to a json format
            $.each(cats, function(i,obj){
              data_serve.push({"cat_id": obj});
            });
            //Switch for the category buttons
            var swtch_var = $(this).attr('data-on');

                if(swtch_var == 0)
                {
                  swtch_var = 1;
                }
                else
                {
                  swtch_var = 0;
                }
                $(this).attr('data-on',swtch_var);  

loadUsers($('#frmSearchUsers'),'',data_serve);


           e.preventDefault();
        });
    });
});

This is how the $_GET parameters are received:

    public function searchUser() 
    {
     $cat_id = (isset($_GET['data_serve']) && !empty($_GET['data_serve'])) ? $_GET['data_serve'] : null;
$search_filter = (isset($_GET['search_filter']) && !empty($_GET['search_filter'])) ? $_GET['search_filter'] : null;

     $products = $this->model->GetAllCustomerFilter($search_filter, $cat_id);
    return $products;
    }

Answer 1

When I changed from the mysqli_functions to PDO with prepared statements, I had the same question. I found the answers to this question to be very usefull: https://stackoverflow.com/questions/134099/are-pdo-prepared-statements-sufficient-to-prevent-sql-injection.

Let's say you have messaging functionality and someone types SQL injection code in the title field when sending a message. You use a prepared statement, and the SQL injection code gets saved as the title. the SQL injection code didn't get executed, you have been saved from 1st order SQL injection.

Now let's say you have a function to reply to messages that uses the previous message title and adds "RE: " before it. If this reply function assumes the new title is safe since it's not coming from a user input field and doesn't use a prepared statement on the reply title, the SQL injection code will get executed. This is called 2nd order SQL injection.

You're 1st order safe if you use prepared statements for queries requiring user input. You're 2nd order safe if you use prepared statements for every query.

Answer 2

You're still vulnerable to SQL Injection. Take another look at loadProductsListPerCat. This function receives the data_serve parameter, sends its contents to the server which in turn puts it straight into an SQL query as the cat IDs:

$.each(data_serve, function(i,obj)
{
    data_fed[i] = obj.cat_id;
});
var raw = data_fed.join();
var encoded = encodeURIComponent(raw);
var formData =  "data_serve="+encoded+"&"+ $(form).serialize(); 
var requestURL = 'searchUsers';
 $.ajax({
        url: requestURL,
        data: formData,
...

Remember that server code should never trust incoming data as safe, even if you wrote the front end (website) code.

If I wanted to inject SQL into your queries, I would go on your website and watch what legitimate HTTP requests look like. Once I know the URL and parameters sent by your AJAX call, I would craft my custom request but replace the valid cat IDs with SQL. I don't even need to use your website to make these requests to your server so good website code does not protect you.

Addendum

One simple fix: in GetAllCustomerFilter; if your cat_id DB field holds integers, use:

$ids_array = explode(',',$cat_id);
foreach ($ids_array as &$id): // "&" allows us to modify $ids_array in the loop
    $id = (int)$id; //cast as integer
endforeach;
$cat_id = implode(',',$ids_array);

Now you can safely put $cat_id in your query. Suppose an attacker sent in a dangerous parameter as the $cat_id such as 1,2,3,5) OR 1; DROP DATABASE myDb; The code below would transform these ids from the dangerous:

Array
(
    [0] => 1
    [1] => 2
    [2] => 3
    [3] => 5) OR 1; DROP DATABASE myDb;
)

Into the safe ids:

Array
(
    [0] => 1
    [1] => 2
    [2] => 3
    [3] => 5
)

Answer 3

For situations where Patrick's example doesn't work because the thing you need many of aren't numbers:

$ids_array = explode(',',$cat_id);

$slots = [];
$bindings = [];
foreach ($ids_array as $id) {
    $slots[] = '?';
    $bindings[] = $id;
}
$slotString = implode(',', $slots);

$stmt = $this->db->prepare("SELECT * from users_cards WHERE cat_id IN ($slotString) AND username LIKE ? ORDER BY id DESC LIMIT 60";

foreach ($bindings as $index => $binding) {
    $stmt->bindParam($index, $binding);
}

$stmt->bindParam($index + 1, $search);
if($stmt->execute()) {
    $products =  $stmt->fetchAll(PDO::FETCH_ASSOC);
}

This results in a $slotString that looks like ?,?,?, which will be bound to the ids (or potentially strings if you needed them to be) and therefore properly protected from injection.