From 934a8a1156f20807c1c132b451c452d39569d1fe Mon Sep 17 00:00:00 2001
From: Jonathan Druart <jonathan.druart@biblibre.com>
Date: Tue, 10 Jul 2012 16:00:54 +0200
Subject: [PATCH] Bug 7955: Followup : Check the syspref value (avoid sql
 injection)

Signed-off-by: Kyle M Hall <kyle@bywatersolutions.com>

Works as expected. Fields with disallowed characters do not show up.
Added 'if $debug' to an pseudo-unconditional warn.

Signed-off-by: Paul Poulain <paul.poulain@biblibre.com>
---
 C4/Members/Statistics.pm | 17 ++++++++++++++---
 members/statistics.pl    |  5 +++--
 2 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/C4/Members/Statistics.pm b/C4/Members/Statistics.pm
index 302259c1c3..9f26b86a25 100644
--- a/C4/Members/Statistics.pm
+++ b/C4/Members/Statistics.pm
@@ -40,6 +40,18 @@ BEGIN {
     );
 }
 
+
+our $fields = get_fields();
+
+sub get_fields {
+    my $r = C4::Context->preference('StatisticsFields') || 'location|itype|ccode';
+    unless ( $r =~ m/^(\w|\d|\||-)+$/) {
+        warn "Members/Statistics : Bad value for syspref StatisticsFields" if $debug;
+        $r = 'location|itype|ccode';
+    }
+    return $r;
+}
+
 =head2 construct_query
   Build a sql query from a subquery
   Adds statistics fields to the select and the group by clause
@@ -47,10 +59,9 @@ BEGIN {
 sub construct_query {
     my $count    = shift;
     my $subquery = shift;
-    my $fields = C4::Context->preference('StatisticsFields') || 'location|itype|ccode';
     my @select_fields = split '\|', $fields;
-    my $query = "SELECT COUNT(*) as count_$count";
-    $query .= ", " . C4::Context->dbh->quote( $_ ) for @select_fields;
+    my $query = "SELECT COUNT(*) as count_$count,";
+    $query .= join ',', @select_fields;
 
     $query .= " " . $subquery;
 
diff --git a/members/statistics.pl b/members/statistics.pl
index 8551ec8f9b..7125560f0e 100755
--- a/members/statistics.pl
+++ b/members/statistics.pl
@@ -58,7 +58,7 @@ foreach my $key ( keys %$borrower ) {
 }
 
 # Construct column names
-my $fields = C4::Context->preference('StatisticsFields') || 'location|itype|ccode';
+my $fields = C4::Members::Statistics::get_fields();
 our @statistic_column_names = split '\|', $fields;
 our @value_column_names = ( 'count_precedent_state', 'count_total_issues_today', 'count_total_issues_returned_today' );
 our @column_names = ( @statistic_column_names, @value_column_names );
@@ -70,6 +70,7 @@ my $total_issues_returned_today = GetTotalIssuesReturnedTodayByBorrower( $borrow
 my $r = merge (
     @$precedent_state, @$total_issues_today, @$total_issues_returned_today
 );
+
 add_actual_state( $r );
 my ( $total, $datas ) = build_array( $r );
 
@@ -194,7 +195,7 @@ sub merge {
         for my $ch ( @r ) {
             $exists = 1;
             for my $cn ( @statistic_column_names ) {
-                if ( not $ch->{$cn} eq $h->{$cn} ) {
+                if ( $ch->{$cn} and not $ch->{$cn} eq $h->{$cn} ) {
                     $exists = 0;
                     last;
                 }
-- 
2.39.5