1

I have a module which allows admins to add some users (maybe multiple) to a group.

PHP

$user_id  = $_POST["user_id"]; //this can be an array
$group_id = $_POST["group_id"];

$sql = "INSERT IGNORE INTO users_groups (user_id, group_id) 
        VALUES ($user_id[0],$group_id)";

for ( $i = 1; $i < count($user_id); $i++) 
{
    $sql .= ", ($user_id[$i],$group_id)"
}

As you can see the sql-query depends on how much users the admin has selected.

How can I use this code with prepared statements because the post-variables come from a user (SQL injections...)

UPDATE

I have two selectboxes:

  • one for the user selection (multiple selection is possible -> array): $_POST["user_id"]
  • one for the group selection (only one selection is possible): $_POST["group_id"]

And now I want a prepared SQL statement for inserting user_id and group_id to the many-to-many-table (users_groups). The problem is that the number of values which have to be inserted can change (depending how much users the admin has selected in the selectbox).

I want to change the prepared query depending on how much users the admin has selected.

For example:

  • the admin selected two users -> sql: INSERT IGNORE INTO users_groups (user_id, group_id) VALUES ($user_id[0],$group_id), ($user_id[1],$group_id)
  • the admin selected four users -> sql: INSERT IGNORE INTO users_groups (user_id, group_id) VALUES ($user_id[0],$group_id), ($user_id[1],$group_id), ($user_id[2],$group_id), ($user_id[3],$group_id)

My question: How can I do this automaticaly and with prepared sql statements because I dont want to have like 10 times if(count($user_id) == number) {...?

UPDATE 2 If I would do this manually the code would look like this:

$sql = $db->prepare("INSERT IGNORE INTO users_groups (user_id, group_id) VALUES (?, ?)"); 
$sql->bind_param('ii', $user_id[0], $group_id);

UPDATE 3 To check whether there are only integers in the post variables:

$user_id = filter_input_array(INPUT_POST, 'user_id', FILTER_SANITIZE_NUMBER_INT);
$user_id = abs($user_id);

$group_id = filter_input(INPUT_POST, 'group_id', FILTER_SANITIZE_NUMBER_INT);
$group_id = abs($group_id);
Klipp Ohei
  • 385
  • 6
  • 16
  • What driver are you using? Have you looked at any of threads on this topic? Example #1: http://stackoverflow.com/questions/60174/how-can-i-prevent-sql-injection-in-php?rq=1 – chris85 Jul 22 '15 at 21:15
  • This is one of those cases where prepared statements don't work awfully well. That said, it looks like you're working with just integers (or an array of integers). It should be possible to sanitise that effectively with PHP's `filter_input_array()` run on the `POST` input, and then you don't need a prepared statement at all. See [filter_input_array()](http://php.net/manual/en/function.filter-input-array.php) –  Jul 22 '15 at 21:47
  • You can define the `$sql` statement and then do the binding in the for loop for each user/group pair. – Maximus2012 Jul 22 '15 at 21:48
  • Is this PDO? What are you using to talk to MySQL? – Brad Jul 22 '15 at 21:53

1 Answers1

2

To create the SQL statement, you can use this to generate as many placeholders as you will need for all of your user_id values.

$user_ids  = $_POST["user_id"]; //this can be an array
$group_id = $_POST["group_id"];
$placeholders = implode(', ', array_fill(0, count($user_ids), "(?, ?)"));    
$sql = "INSERT IGNORE INTO users_groups (user_id, group_id) VALUES $placeholders";

Then to bind the values, if you are using pdo, something like this should work:

$stmt = $pdo->prepare($sql);
$i = 1;
foreach ($user_ids as $user_id) {
    $stmt->bindValue($i++, $user_id);
    $stmt->bindValue($i++, $group_id);
}

Or if you are using mysqli, you could try this approach using reflection (taken from a user note in the php docs here) the note does state that this needs PHP 5.3+, which hopefully you do have.

$stmt = $mysqli->prepare($sql);
$values[] = str_repeat('ii', count($user_ids));
foreach ($user_ids as $user_id) {
    $values[] = $user_id;
    $values[] = $group_id;
}
$ref = new ReflectionClass('mysqli_stmt');
$method = $ref->getMethod("bind_param");
$method->invokeArgs($stmt, $values);
$stmt->execute(); 

Or, because I'm stubborn and the thing from the php docs user note doesn't seem to work, which after reading more I don't see how it could work, considering the php doc for ReflectionMethod::invokeArgs specifically says

Note: If the function has arguments that need to be references, then they must be references in the passed argument list.

then using call_user_func_array could possibly work.

$stmt = $mysqli->prepare($sql);
$type = str_repeat('ii', count($user_ids));
foreach ($user_ids as $user_id) {
    $values[] = $user_id;
    $values[] = $group_id;
}
foreach ($values as $key => $value) {
    $value_references[$key] = &$values[$key];
}
call_user_func_array('mysqli_stmt_bind_param', 
    array_merge(array($stmt, $type), $value_references));

But what a pain. I really like pdo.

Don't Panic
  • 41,125
  • 10
  • 61
  • 80
  • I used now your part with the `$placeholder` but I get problems with the part below. I normaly use bind_param like this: `$sql->bind_param('ii', $user_id[0], $group_id);` and with multiple user_id's it would look like this: `$sql->bind_param('iiii', $user_id[0], $group_id, $user_id[1], $group_id);` And now I dont know how to implement this in the foreach loop. – Klipp Ohei Jul 22 '15 at 22:16
  • @KlippOhei Well, I wrote this answer before knowing you were using mysqli. Unfortunately, this will not work using mysqli because of its different approach to binding parameters. I have been using pdo rather than mysqli for a while (partially because I think it's much easier to work with prepared statements in pdo because of issues like this) so I can't think of how to handle this in mysqli at the moment. I think it's still possible, but kind of a pain. I will see if I can find some of my old code where I did this and update the answer. – Don't Panic Jul 22 '15 at 22:21
  • Maybe I can try to not use prepared statements because the values which the user has to post to the script are just numbers so I can check whether the posted variables are numbers (with `is_numeric`) and if not, exit the script. Would this be safe against SQL injections? – Klipp Ohei Jul 22 '15 at 22:24
  • It sounds like it would probably be ok. Using filter_input_array as Hobo Sapiens suggested in the comment on your question would help with that. If you still want to try with the prepared statement, an approach like in [this answer](http://stackoverflow.com/a/16120923/2734189) will probably work, with some modification. – Don't Panic Jul 22 '15 at 22:31
  • I'll try your new version in a few minutes. :) Would my Update 3 (first post) work against SQL injections? – Klipp Ohei Jul 22 '15 at 22:53
  • Well, I don't believe that's quite how filter_input_array works. Check out the example here: http://php.net/manual/en/function.filter-input-array.php Also, filter_input_array will return an array, and using abs on that will not do what you have in mind. – Don't Panic Jul 22 '15 at 22:58
  • I tried now your method for mysqli but I get a bunch of errors on this line: `$method->invokeArgs($stmt, $values);` Errors: `Warning: Parameter 2 to mysqli_stmt::bind_param() expected to be a reference, value given` and `Fatal error: Uncaught exception 'ReflectionException' with message 'Invocation of method mysqli_stmt::bind_param() failed' in ... Stack trace: #0 ...: ReflectionMethod->invokeArgs(Object(mysqli_stmt), Array) #1 {main} thrown in ...` – Klipp Ohei Jul 22 '15 at 23:08