Bug 19061: Avoid SQL Injection vulnerability
authorColin Campbell <colin.campbell@ptfs-europe.com>
Tue, 8 Aug 2017 10:47:40 +0000 (11:47 +0100)
committerJonathan Druart <jonathan.druart@bugs.koha-community.org>
Tue, 15 Aug 2017 15:17:43 +0000 (12:17 -0300)
Embedding values in the SQL statement allows the passing of values
that would normally be rejected resulting in mysql errors
variables should always be passed via placeholders and
the execute call

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>

Signed-off-by: Jonathan Druart <jonathan.druart@bugs.koha-community.org>

reports/cash_register_stats.pl

index 3bd8cad..9ce0dca 100755 (executable)
@@ -66,23 +66,26 @@ if ($do_it) {
     $toDate   = output_pref({ dt => eval { dt_from_string($input->param("to")) } || dt_from_string,
             dateformat => 'sql', dateonly => 1 }); #for sql query
 
-    my $whereTType = '';
+    my $whereTType = q{};
+    my @extra_params; # if we add conditions to the select we need extra params
 
     if ($transaction_type eq 'ALL') { #All Transactons
-        $whereTType = '';
+        $whereTType = q{};
     } elsif ($transaction_type eq 'ACT') { #Active
-        $whereTType = " accounttype IN ('Pay','C') AND ";
+        $whereTType = q{ AND accounttype IN ('Pay','C') };
     } else { #Single transac type
         if ($transaction_type eq 'FORW') {
-            $whereTType = " accounttype = 'FOR' OR accounttype = 'W' AND ";
+            $whereTType = q{ AND accounttype = 'FOR' OR accounttype = 'W' };
         } else {
-            $whereTType = " accounttype = '$transaction_type' AND ";
+            $whereTType = q{ AND accounttype = ? };
+            push @extra_params, $transaction_type;
         }
     }
 
-    my $whereBranchCode = '';
+    my $whereBranchCode = q{};
     if ($manager_branchcode ne 'ALL') {
-        $whereBranchCode = "AND m.branchcode = '$manager_branchcode'";
+        $whereBranchCode = q{ AND m.branchcode = ?};
+        push @extra_params, $manager_branchcode;
     }
 
 
@@ -98,13 +101,13 @@ if ($do_it) {
         LEFT JOIN branches br ON (br.branchcode = m.branchcode )
         LEFT JOIN items i ON (i.itemnumber = al.itemnumber)
         LEFT JOIN biblio bi ON (bi.biblionumber = i.biblionumber)
-        WHERE $whereTType
-        CAST(al.date AS DATE) BETWEEN ? AND ?
+        WHERE CAST(al.date AS DATE) BETWEEN ? AND ?
+        $whereTType
         $whereBranchCode
         ORDER BY al.date
     ";
-    my $sth_stats = $dbh->prepare($query) or die "Unable to prepare query" . $dbh->errstr;
-    $sth_stats->execute($fromDate, $toDate) or die "Unable to execute query " . $sth_stats->errstr;
+    my $sth_stats = $dbh->prepare($query) or die "Unable to prepare query $dbh->errstr";
+    $sth_stats->execute($fromDate, $toDate, @extra_params) or die "Unable to execute query $sth_stats->errstr";
 
     my @loopresult;
     my $grantotal = 0;