Prevent SQL injection for bulk prepared raw query using Laravel

Question

This is my query:

$last = count($list)-1;

$sql = 'INSERT INTO table (col1, col2, col3) VALUES ';
for($i=0; $i<$last; $i++) {
    $sql .= '("'. $list[$i]['col1'] .'", "'. $list[$i]['col2'] .'", '. $list[$i]['col3'] .'), ';
}
$sql .= '("'. $list[$last]['col1'] .'", "'. $list[$last]['col2'] .'", '. $list[$last]['col3'] .') ';
$sql .= 'ON DUPLICATE KEY UPDATE ';
for($i=0; $i<$last; $i++) {
    $sql .= 'col3 = '. $list[$i]['col3'] .', ';
}
$sql .= 'col3 = '. $list[$last]['col3'];

DB::statement($sql);

Query without PHP:

INSERT INTO table (col1, col2, col3) VALUES
    ( $list[$i]['col1'] , $list[$i]['col2'] , $list[$i]['col3'] ),
    ...
    ( $list[$i]['col1'] , $list[$i]['col2'] , $list[$i]['col3'] )

ON DUPLICATE KEY UPDATE
    col3 = $list[$i]['col3'],
    ...
    col3 = $list[$i]['col3'];

I've looked at this and this. It's somewhat unclear to me how I should be doing it. Am I supposed to do it like this?

DB::connection()->getPdo()->quote($sql);

I've also read you can do something along these lines:

DB::escape($sql)

What is the best and easiest way to prevent SQL injection for the above query using Laravel?

Use prepared/parameterized queries https://laravel.com/docs/5.2/database#running-queries — JimL, Jan 06 '16 at 13:16
Can you show me an example of how I would do it for the above query? Also I'm using Laravel 4.2 if that matters. — Howard, Jan 06 '16 at 13:18
It should be pretty self explanatory, you can find the DB::insert for 4.2 here https://laravel.com/docs/4.2/database#running-queries Only change you need to do is to change `array(1, 'Dayle')` to a variable, initialize it as an array before your query and push the values you want added into it (in the correct order). — JimL, Jan 06 '16 at 13:25
I've already looked at that. It's unclear to me how I would write a large prepared query that way though. — Howard, Jan 06 '16 at 13:28
Just have it the exact same as you have it now, except you change all the variables in the query string into placeholders `?` and instead push the variables into an array that you later send into the `DB::insert` method. — JimL, Jan 06 '16 at 13:29
I can't just use an insert because I want it to update and only insert if the row doesn't exist. — Howard, Jan 06 '16 at 13:31
I am though. If you look at the end of my query code it says. DB::statement($sql); Like how would I write this query? — Howard, Jan 06 '16 at 13:33
Like if it were a single line query I could do it easily. I don't know how to do this since it's bulk prepared. — Howard, Jan 06 '16 at 13:44
I told you spesifically how to solve this in http://stackoverflow.com/questions/34634068/prevent-sql-injection-for-bulk-prepared-raw-query-using-laravel?noredirect=1#comment57013584_34634068 If you have any problems doing that I'd create a new question with your best code attempt — JimL, Jan 06 '16 at 14:37
@JimL Your link was really helpful. I solved it and posted the answer below. Thank you. — Howard, Jan 06 '16 at 15:00
@rotaercz i've noticed you need a lot of spoon feeding and don't use `foreach` appropriately. You need to try and do some research or buy a book. — Ash, Jan 06 '16 at 15:37
@ash Thanks I guess? I didn't use foreach because it's slightly slower than a manual for loop. — Howard, Jan 06 '16 at 15:42
@rotaercz that's not true for php7 - besides you're using a framework which is slower than procedural - it's stupid reasoning — Ash, Jan 06 '16 at 15:55
I'm not using php7. I'm using php5.5. My reasoning is to get as much speed for the above code. Not sure what your problem is. — Howard, Jan 06 '16 at 16:00
I dont have aproblem, but other people will come here and think using `for` is better than `foreach` because of speed lol. yet you're using a framework so your argument on "speed" is irrelevent - if you want speed use C and compile it as a PHP extension like the Phalcon framework :/. Otherwise cleaner code is better, foreach is cleaner and clearer than for loops. — Ash, Jan 06 '16 at 16:37

score 3 · Answer 1 · edited May 23 '17 at 12:15

3

The best and easiest way to prevent SQL injection is to use parameterised queries. How to do it for bulk inserts was answered many times already, e.g. here: Insert multiple rows with PDO prepared statements

EDIT:

To make it crystal clear, to execute parametrised query in Laravel you do

DB::insert($query, $params);

where in pure PDO answers it is

$stmt = DB::getInstance()->prepare($query);
$stmt->execute($params);

The question how to prepare the $query and $params for bulk inserts has been answered already, massively discussed for pros and cons, and some alternative solutions for edge case scenarios emerged. Please read them and understand how it works.

edited May 23 '17 at 12:15

Community

1
1

answered Jan 06 '16 at 13:22

Alex Blex

34,704
7
48
75

That link is helpful but I'm asking if there's a right way to do it with Laravel. – Howard Jan 06 '16 at 13:23
1

Since you are asking about **raw** queries, framework doesn't really matter, as you make no use of its model layer. – Alex Blex Jan 06 '16 at 13:26
I don't think the above query can be written in Laravel Eloquent. I tried but it becomes a for loop which hits the database n number of times based on how many items are in $cardQueryList so I changed it to a prepared raw so it only hits the database once. – Howard Jan 06 '16 at 13:30
That's a very good optimisation step, and I gave you 2 links to answer the question how to do it safely. – Alex Blex Jan 06 '16 at 13:35
Optimally I'd like to do it in the way that @JimL mentioned. It's unclear to me how though I would do it for a bulk statement like this. I'd like to keep things in Laravel as much as possible. – Howard Jan 06 '16 at 13:49
You're using MySQL. You're also using `ON DUPLICATE KEY UPDATE`. You're also constructing the actual query from ground up - there is no Laravel way here, you're already outside of the framework's way of doing things. And bulk inserts are not what you want, you want prepared statement, executed multiple times with different parameters and flushed within a single transaction so you can achieve high write speed. You can either take the answer from Alex or ignore it and do everything the bad way (for no apparent reason). – Mjh Jan 06 '16 at 14:09
I'm not ignoring his answer. His answer is a last resort for me. I'm currently working on my answer based on this. https://laravel.com/docs/4.2/database#running-queries as I want it to go through Laravel as much as possible. And no answer currently shows me how to do this via Laravel. If you know how to do it purely via Laravel Eloquent I'll pick yours as the best answer. – Howard Jan 06 '16 at 14:15
@rotaercz, I have updated the answer to make it a bit more clear. There is no magic in Eloquent, it is still old good PDO inside, with some fancy styling on top. – Alex Blex Jan 06 '16 at 14:34
If you have dynamically constructed query, as you do have, then you can't use Eloquent. And as Alex said, Eloquent is good old PDO with some sugar on top. Your use case is different and there's nothing Laravel can help you with except give you the PDO object (based on .env settings). – Mjh Jan 06 '16 at 14:41
I'm aware under the hood it's all PHP. Here's an upvote for the suggestion though. – Howard Jan 06 '16 at 14:54
Hi is Model::create(array) method of elequent is safe for storing data. – Muhammad Usama Mashkoor Aug 20 '17 at 20:01

Howard · Accepted Answer · 2016-01-06T17:22:07.550

0

I think this is it unless someone can do it with pure Eloquent. Hopefully this helps someone else trying to do something similar.

Since I am using col1 + col2 as a key I had to create a composite unique key like this:

CREATE UNIQUE INDEX col1_col2 ON table (col1, col2)

And here it is for posterity:

$last = count($list)-1;

$sql = 'INSERT INTO table (col1, col2, col3) VALUES ';
$values = [];

for($i=0; $i<$last; $i++) {
    $sql .= '( ?, ?, ? ), ';
    $values[] = $list[$i]['col1'];
    $values[] = $list[$i]['col2'];
    $values[] = $list[$i]['col3'];
}
$sql .= '( ?, ?, ? ) ';
$values[] = $list[$last]['col1'];
$values[] = $list[$last]['col2'];
$values[] = $list[$last]['col3'];

$sql .= 'ON DUPLICATE KEY UPDATE ';
$sql .= 'col3 = VALUES(col3)';

DB::statement($sql, $values);

Cheers!

edited Jan 06 '16 at 17:22

answered Jan 06 '16 at 14:52

Howard

3,648
13
58
86

Yes, you get it right. Few things to consider tho: `row1` kind of names may be a bit confusing for someone else who you are trying to help. It really should be `filed1` or `column1`. Please ensure your `ON DUPLICATE` stuff does exactly what you expect, otherwise you will make more harm than good to ones who trying to do something similar. – Alex Blex Jan 06 '16 at 15:54
I updated to col1, col2, etc. I'm trying to use col1 + col2 as the key. I'll go test some more. – Howard Jan 06 '16 at 15:58
@AlexBlex They don't care, they just want spoon feeding. "Here is my problem - please write the code for me so it works" RTFM is not something they can handle – Ash Jan 06 '16 at 15:59
@ash You're aware that I answered my own question and the answer I posted here is mine? What is your problem? If you don't want to help don't post. – Howard Jan 06 '16 at 16:02
@rotaercz i was commenting to Alex Blex. I am willing to help, guide and metor through showing you and others how to find the answers for yourself. So you will improve. I absolutely do not spoon feed, that teaches nothing. – Ash Jan 06 '16 at 16:34
I'm done here and not going to help you any further because of your down-right ignorance. – Ash Jan 06 '16 at 16:41
@AlexBlex: You were right. The update wasn't working the way I wanted. I've updated my answer accordingly so everything works perfectly now. – Howard Jan 06 '16 at 17:25
Hi is Model::create(array) method of elequent is safe for storing data. – Muhammad Usama Mashkoor Aug 20 '17 at 20:01

score 0 · Answer 3 · answered Jan 06 '16 at 15:05

looks like an issue when using raw queries. Found a great article and wanted to pass it along for anyone else that may need it.http://fideloper.com/laravel-raw-queries

Laravel does use prepared statements as barry mentioned, but it looks like you need to specifically pass variables in an array of bindings for this to happen with raw queries.

 $someVariable = Input::get("some_variable");

$results = DB::select( DB::raw("SELECT * FROM some_table WHERE some_col = :somevariable"), array(
   'somevariable' => $someVariable,
 ));

Prevent SQL injection for bulk prepared raw query using Laravel

3 Answers3