From a08c09884ec8799f2835b6a63ed10833bd927fbf Mon Sep 17 00:00:00 2001
From: Julian Maurice <julian.maurice@biblibre.com>
Date: Mon, 20 Aug 2012 08:57:42 +0200
Subject: [PATCH] Bug 7678: follow-up to pass QA

Check input parameters to avoid SQL injection
Rewrite a for loop for readability

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
All tests pass, perlcritic is unchanged.

Test plan
- Verify ExtendedPatronAttributes hides/shows new patron attribute
  search criteria
- Verify patron attribute show up correctly
  - pull down list for attributes linked to authorized values
  - input field for other attributes
- Verify search works correctly
Signed-off-by: Jared Camins-Esakov <jcamins@cpbibliography.com>
---
 reports/borrowers_stats.pl | 34 +++++++++++++++++++++++++++-------
 1 file changed, 27 insertions(+), 7 deletions(-)

diff --git a/reports/borrowers_stats.pl b/reports/borrowers_stats.pl
index e2defa9cbc..eca9b3abc2 100755
--- a/reports/borrowers_stats.pl
+++ b/reports/borrowers_stats.pl
@@ -30,6 +30,7 @@ use C4::Acquisition;
 use C4::Output;
 use C4::Reports;
 use C4::Circulation;
+use C4::Members::AttributeTypes;
 use C4::Dates qw/format_date format_date_in_iso/;
 use Date::Calc qw(
   Today
@@ -180,6 +181,25 @@ sub calculate {
 # extract parameters
 	my $dbh = C4::Context->dbh;
 
+    # check parameters
+    my @valid_names = qw(categorycode zipcode branchcode sex sort1 sort2);
+    my @attribute_types = C4::Members::AttributeTypes::GetAttributeTypes;
+    if ($line =~ /^patron_attr\.(.*)/) {
+        my $attribute_type = $1;
+        return unless (grep {$attribute_type eq $_->{code}} @attribute_types);
+    } else {
+        return unless (grep /^$line$/, @valid_names);
+    }
+    if ($column =~ /^patron_attr\.(.*)/) {
+        my $attribute_type = $1;
+        return unless (grep {$attribute_type eq $_->{code}} @attribute_types);
+    } else {
+        return unless (grep /^$column$/, @valid_names);
+    }
+    return if ($digits and $digits !~ /^\d+$/);
+    return if ($status and (grep /^$status$/, qw(debarred gonenoaddress lost)) == 0);
+    return if ($activity and (grep /^$activity$/, qw(active nonactive)) == 0);
+
     # Filters
     my $linefilter;
     given ($line) {
@@ -206,7 +226,7 @@ sub calculate {
     }
 
     my @loopfilter;
-    for (my $i = 0; $i <= scalar @$filters; $i++) {
+    foreach my $i (0 .. scalar @$filters) {
         my %cell;
         if ( @$filters[$i] ) {
             if ($i == 3 or $i == 4) {
@@ -248,7 +268,7 @@ sub calculate {
 	my $linefield;
 
     my $line_attribute_type;
-    if ($line  =~/^patron_attr.(.*)$/) {
+    if ($line  =~/^patron_attr\.(.*)$/) {
         $line_attribute_type = $1;
         $line = 'borrower_attributes.attribute';
     }
@@ -304,11 +324,11 @@ sub calculate {
         $column = 'borrower_attributes.attribute';
     }
 
-	if (($column =~/zipcode/) and ($digits)) {
-		$colfield = "left($column,$digits)";
-	} else {
-		$colfield = $column;
-	}
+    if (($column =~/zipcode/) and ($digits)) {
+        $colfield = "left($column,$digits)";
+    } else {
+        $colfield = $column;
+    }
 
     my $strsth2;
     my @strparams2; # bind parameters for the query
-- 
2.39.5