Permalink
Browse files

Bug 6452 : Fixing missing sql placeholders

  • Loading branch information...
1 parent e4ebd27 commit 4dc34b74fd19fafde9105b40c281f18aab542c59 @ranginui committed Jun 3, 2011
Showing with 2 additions and 2 deletions.
  1. +2 −2 C4/Acquisition.pm
View
@@ -930,9 +930,9 @@ sub GetParcels {
sum(quantity) AS itemsexpected,
sum(quantityreceived) AS itemsreceived
FROM aqorders LEFT JOIN aqbasket ON aqbasket.basketno = aqorders.basketno
- WHERE aqbasket.booksellerid = $bookseller and datereceived IS NOT NULL
+ WHERE aqbasket.booksellerid = ? and datereceived IS NOT NULL
";
-
+ push @query_params, $bookseller;
if ( defined $code ) {
$strsth .= ' and aqorders.booksellerinvoicenumber like ? ';
# add a % to the end of the code to allow stemming.

1 comment on commit 4dc34b7

@ctfliblime

Thanks for the submission. It's definitely cleaner to not to be expanding variables within a query string, and to rely instead on the underlying DBD's quoting utilities used in the parameter passing. At the same time, it's hard to see how this and most other instances of this type of sloppy expansion in Koha are exploitable in a useful manner unless you're enabling mysql_multi_statements => 1 at connect time, which we aren't doing. Well, I suppose in this case you could fudge $bookseller to be something like " 'anything' OR aqbasket.booksellerid IS NOT NULL" to get parcels for all the booksellers instead of just one. I don't know how useful that is, though, in terms of undermining the security considerations. Nevertheless, better to fix it than to find out the hard way it's more of an issue than thought!

Please sign in to comment.