Parameterized query binding in ON clause for a LEFT JOIN in Laravel Eloquent / Query Builder

Question

Let's say I want to show a full list of awards with type="color":

Awards        Type     2013 Winner
======        ====     ===========
Blue Award    color       Tom
Red Award     color
Green Award   color       Dan

To achieve this result I could have a query in Laravel like this:

$year = '2013';

$awards = DB::table('awards')
             ->leftJoin('winners', function($join) use ($year)
                   {
                        $join->on('awards.id','=','winners.award_id');
                        $join->on('winners.year','=',DB::raw("'".$year."'"));
                   }
             ->where('awards.type','color')
             ->get();

If you output the SQL that Laravel generates you will see that only the WHERE clause is parameterized and $year in the ON clause is left vulnerable to sql injection if I get it from an untrusted source. Also the query's caching potential is reduced because $year will change often. Note: In case you were thinking that I just add the second left join condition to the WHERE of the query, these are not the same.

Any ideas on how to get the $year part of the query parameterized?

I've now opened an issue regarding this: https://github.com/laravel/framework/issues/1904 — prograhammer, Jul 19 '13 at 02:50

prograhammer · Accepted Answer · 2015-02-03T15:38:05.437

Here's an odd work-around (didn't want to extend the Builder and JoinClause classes):
Notice: This will break query chaining with -> so notice the where was seperated below.

$query = DB::table('awards')
         ->leftJoin('winners', function($join)
               {
                    $join->on('awards.id','=','winners.award_id');
                    $join->on('winners.year','=',DB::raw('?'));  
               }
         ->setBindings(array_merge($query->getBindings(),array($year)));

$query->where('awards.type','color');

$awards = $query->get();

UPDATE: Taylor added joinWhere, leftJoinWhere... he says that "if you have a function join just use ->where and ->orWhere from within the Closure." I've yet to try this though.

score 1 · Answer 2 · answered Jul 21 '16 at 21:50

Currently you can use $join->where:

$year = '2013';

$awards = DB::table('awards')
         ->leftJoin('winners', 
               function($join) use ($year)
               {
                    $join
                        ->on('awards.id','=','winners.award_id')
                        // "where" instead of "on":
                        ->where('winners.year', '=', $year);
               }
         ->where('awards.type','color')
         ->get();

score -1 · Answer 3 · answered Jul 18 '13 at 21:47

-1

This comes straight from the Laravel docs:

The Laravel query builder uses PDO parameter binding throughout to protect your application against SQL injection attacks. There is no need to clean strings being passed as bindings.

You shouldn't need to sanitize it at all. It should be fine. If you are worried about it though, you can use the Validator class to validate it however you want.

answered Jul 18 '13 at 21:47

searsaw

3,492
24
31

If you look at the part where $year is, you can see I put quotes in there. Anything is accepted there. I tested and I found I can sql inject into that part easily. If you output the SQL you'll see that everywhere parameter binding is used a ? will show, but there is no ? in the ON clause for $year. – prograhammer Jul 18 '13 at 21:53
Why not try `$join->on('winners.year','=', $year)` – searsaw Jul 18 '13 at 22:03
That will add back ticks (grave accents) around the year like this \`2013\` and will throw a mysql error. See also: http://stackoverflow.com/a/16849231/1110941 – prograhammer Jul 18 '13 at 22:16
Jason Lewis' answer in that post http://stackoverflow.com/a/16853305/1110941 might have a clue though...hmmm...thinking... – prograhammer Jul 18 '13 at 22:21
2

Maybe trying using the variable in the function. Like this: `leftJoin('winners', function($join) use ($year)` – searsaw Jul 19 '13 at 02:18
Nope, no good alex. There just doesn't seem to be any code to support bindings for joins in the laravel source. – prograhammer Jul 19 '13 at 02:56
1

I would've said that you'd just need to inject the variable into the scope of the closure (much like @searsaw has shown) and then use DB::raw(). So it'd be `DB::raw($year);`, but make sure you have the `use ($year)` otherwise the variable isn't within the scope of the closure. – Jason Lewis Jul 19 '13 at 07:52
sorry, yes that's what I do to use $year in the closure...but change `DB::raw($year)` to `DB::raw("'2013'")` then output the SQL laravel creates and you'll see the problem. – prograhammer Jul 19 '13 at 16:33
I also edited the question to include the "use ($year)" now...but problem is still there. Thanks for helping though!! – prograhammer Jul 19 '13 at 16:43
Ok Alex, Jason, I updated my solution (work-around) that seems to be alright and doesn't require the need to extend laravel query classes. What do you think? Am I way off track here? – prograhammer Jul 19 '13 at 23:02
1

@David Graham, Will it work if you set the bindings directly in the closure instead of out of it? – searsaw Jul 19 '13 at 23:20
Yeah, by adding `use ($query, $year)` and `query->setBindings(array_merge($query->getBindings(),array($year)));` inside of the closure, this allows me to not break the chaining, but on the downside breaks the uniform look. Plus I'm required to add more stuff in the `use`. Thanks Alex for the thinking on it. – prograhammer Jul 19 '13 at 23:34

Parameterized query binding in ON clause for a LEFT JOIN in Laravel Eloquent / Query Builder

3 Answers3

Linked