Mysql dynamic query [sql injection]

Question

I usually use prepared statements but on the particular page I am using a dynamic query and I can't find a reliable way to prevent SQL injection.

if(isset($_GET['sub_cat'])){

    if($_GET['sub_cat'] != '')
        $conditions[] = 'ad_sub_cat='.$_GET['sub_cat'].'';
}

if(isset($_GET['ad_brand'])){

    if($_GET['ad_brand'] != '')
        $conditions[] = "`ad_brand` LIKE CONCAT('%','".$_GET['ad_brand']."','%') ";
}

if(isset($_GET['min_range'])){

    if($_GET['min_range'] != '')
        $conditions[] = 'ad_price >='.$_GET['min_range'].'';
}

if(isset($_GET['max_range'])){

    if($_GET['max_range'] != '')
        $conditions[] = 'ad_price <='.$_GET['max_range'].'';
}

if(isset($_GET['for_r_s'])){
    
    if($_GET['for_r_s'] != '')
        $conditions[] = 'for_r_s ='.$_GET['for_r_s'].'';
}

$query = "SELECT posts.ID, posts.ad_title, posts.ad_price, posts.ad_location, posts.ad_sub_cat FROM `posts` WHERE ". implode(' AND ', $conditions) ."";

IMSoP · Accepted Answer · 2021-11-06 15:07:17Z

3

You prevent SQL injection the same way you would with any other query: by separating the query, which should always be based on hard-coded values, from the data. The only difference is that you need to build up both the query and the list of data parameters conditionally.

So instead of:

if(isset($_GET['sub_cat'])){

    if($_GET['sub_cat'] != '')
        $conditions[] = 'ad_sub_cat='.$_GET['sub_cat'].'';
}

You should write:

if(isset($_GET['sub_cat'])){

    if($_GET['sub_cat'] != '') {
        $sql_conditions[] = 'ad_sub_cat=:sub_cat';
        $parameters['sub_cat'] = $_GET['sub_cat'];
    }
}

Or if using positional rather than named parameters:

if(isset($_GET['sub_cat'])){

    if($_GET['sub_cat'] != '') {
        $sql_conditions[] = 'ad_sub_cat=?';
        $parameters[] = $_GET['sub_cat'];
    }
}

And so on for all your conditions.

Then at the end, you build up your SQL from $sql_conditions - which has no user input in it at all, so is safe from injection - and pass $parameters to PDO/mysqli to execute the query.

edited Nov 6, 2021 at 15:07

answered Nov 6, 2021 at 14:58

IMSoP

99.7k18 gold badges135 silver badges183 bronze badges

Sign up to request clarification or add additional context in comments.

4 Comments

DB_cont Over a year ago

okay i was wondering if i should do it like this but wanted to ask and see if there is any other solution. thanks best answer have a great day or night

DB_cont Over a year ago

can I ask $get_posts->bind_param(' ', ); how am I going to get s in the first field should i create another $var for that aswell like parameters?

IMSoP Over a year ago

@DB_cont Ah yes, I use PDO not mysqli so didn't think about that. You can build it up at the same time as the parameters (e.g. $placeholder_types .= 's';) or if they're all strings build it at the end with str_repeat('s', count($parameters))

DB_cont Over a year ago

the first one is what i did but might use PDO in the future this is just a beta version :)

Afsal · Accepted Answer · 2021-11-06 15:23:16Z

-1

Use mysqli_real_escape_string before adding the parameter to the query string. For example:

if(isset($_GET['sub_cat']) && $_GET['sub_cat'] != ''){

    $sub_cat = mysqli_real_escape_string($connection, $_GET['sub_cat']);

    $conditions[] = 'ad_sub_cat='.$sub_cat.'';
}

answered Nov 6, 2021 at 15:23

Afsal

346 bronze badges

5 Comments

DB_cont Over a year ago

No. you can still inject even with mysqli_real_escape_string

IMSoP Over a year ago

@DB_cont Under some rare circumstances, yes.

DB_cont Over a year ago

@Afsal well like we learn we should always consider those rare instances because a good hacker will try it and then you will have a problem when your site is compromised

Afsal Over a year ago

@DB_cont If you want to stick with the procedural method instead of PDO, I think this is the recommended and best go. (Corrections are always appreciated :) ) Or you can work around the PDO a little more to generate the desired query using switch statements. This was you will generate and bind the params inside the case based on weightage (an identifier to determine the combination). After all, it is up to the developer how he wants to implement the solution. The query generation can be implemented in various ways.

DB_cont Over a year ago

@Afsal I do appreciate you taking the time to write this answer and I am not saying its wrong just saying its not as good as the first answer but still you helped me mate and i hope you have a nice day

Collectives™ on Stack Overflow

Mysql dynamic query [sql injection]

2 Answers 2

4 Comments

5 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

2 Answers 2

4 Comments

5 Comments

Your Answer

Sign up or log in

Post as a guest

Related