From 1c2ee5903cfe19379d20a847558fb0c176580cc0 Mon Sep 17 00:00:00 2001
From: Colin Campbell <colin.campbell@ptfs-europe.com>
Date: Tue, 8 Aug 2017 11:47:40 +0100
Subject: [PATCH] Bug 19061: Avoid SQL Injection vulnerability

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 | 25 ++++++++++++++-----------
 1 file changed, 14 insertions(+), 11 deletions(-)

diff --git a/reports/cash_register_stats.pl b/reports/cash_register_stats.pl
index 3bd8cadbff..9ce0dcaa4d 100755
--- a/reports/cash_register_stats.pl
+++ b/reports/cash_register_stats.pl
@@ -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;
-- 
2.39.5