Dynamic SQL statement based on foreach

Question

How would I get this to work, because I am just getting errors right now.

$_GET['providers'] is an array of DB column names, which I am checking if = 1 in the below query.

foreach ($_GET['providers'] as $providers) {
  $statement = "AND ".$providers."= '1' ";
}
$sql = "select * from users where user_id ='1' ".$statement." ";
$result = mysqli_query($con, $sql);
$row = mysqli_fetch_assoc($result);
if(isset($row['user_id'])){
 echo "It worked";
}

Sorry for forgetting to say, it's saying $statement is undefined. The one in the SQL query. — TaLeNT, Jun 25 '18 at 17:56
Oh so many issues with this. First, `$statement` needs to be defined before your foreach. That way if `providers` is empty, the statement already exists. Second, you're overwriting the variable in each loop. You need to concatenate, or build up an array that you could implode later. Third, as seen with the highlighting, your statement attempt is built wrong. — aynber, Jun 25 '18 at 18:00
This is unrelated to your problem, but if this isn't a simplified example, you're *extremely* vulnerable to [SQL injection](https://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php) here. — Ray, Jun 25 '18 at 18:01
@aynber also, i just forgot to define statement outside first, sorry. — TaLeNT, Jun 25 '18 at 18:04
If you do a `var_dump($statement)` after the loop, it should make the issues clearer. — Ray, Jun 25 '18 at 18:06
@TaLeNT You seem have asked almost the same question 3 times. — user3783243, Jun 25 '18 at 19:03

score 0 · Answer 1 · answered Jun 25 '18 at 18:17

You should use a whitelist to check if the $providers are known column names. You then should concatenate the $statement, otherwise you overwrite that variable on every iteration.

$statement = '';
$columns = array('known', 'columns', 'go', 'here');
foreach ($_GET['providers'] as $providers) {
    if(in_array($providers, $columns)) {
           $statement .= " AND $providers = 1 ";
    }
}
$sql = "select user_id from users where user_id =1 $statement limit 1";
$result = mysqli_query($con, $sql);
$row = mysqli_fetch_assoc($result);
if(isset($row['user_id'])){
    echo "It worked";
}

You also shouldn't use * unless you really want every column. If you just want to see if a row is returned you also can use limit 1 because you don't care about other rows.

score 0 · Answer 2 · answered Jun 25 '18 at 18:18

You are overwriting $statement every time the loop is running.

$statement = "";
foreach ($_GET['providers'] as $providers) {
    $statement .= "AND ".$providers."= '1' "; // note the ".=" to append
}
$sql = "select * from users where user_id ='1' ".$statement." ";
// to debug: echo "Query :: $sql";
$result = mysqli_query($con, $sql);
$row = mysqli_fetch_assoc($result);
if(isset($row['user_id'])){
    echo "It worked";
}

Dynamic SQL statement based on foreach

2 Answers2