From: Jason Stephenson <jstephenson@mvlc.org>
Date: Sat, 21 Sep 2013 20:51:41 +0000 (-0400)
Subject: Make a little fix to prevent SQL injection with the library option.
X-Git-Url: https://old-git.evergreen-ils.org/?a=commitdiff_plain;h=d8159d92ec2286b6f5c8af90a53f1c6c9e59a2ca;p=working%2FEvergreen.git

Make a little fix to prevent SQL injection with the library option.

This commit is likely to get squashed away before I add a pull request
tag or list the branch on Launchpad.

Signed-off-by: Jason Stephenson <jstephenson@mvlc.org>
---

diff --git a/Open-ILS/src/support-scripts/Marque.pm.in b/Open-ILS/src/support-scripts/Marque.pm.in
index 7d187f3437..50fc8c923e 100644
--- a/Open-ILS/src/support-scripts/Marque.pm.in
+++ b/Open-ILS/src/support-scripts/Marque.pm.in
@@ -326,6 +326,20 @@ sub new {
     $self->{acpClass} = Fieldmapper::class_for_hint('acp');
     $self->{sreClass} = Fieldmapper::class_for_hint('sre');
 
+    # Make an arrayref of shortname ids if the library option was
+    # specified:
+    $self->{libs} = [];
+    if ($Marque::config->option_value('library')) {
+        # This is done not only for speed, but to prevent SQL injection.
+        my $sth = $self->{handle}->prepare('select id from actor.org_unit where shortname=any(?::text[])');
+        if ($sth->execute($Marque::config->option_value('library'))) {
+            my $r = $sth->fetchall_arrayref();
+            my @ids = map {$_->[0]} @{$r};
+            $self->{libs} = \@ids;
+            $sth->finish();
+        }
+    }
+
     bless $self, $class;
     return $self;
 }
@@ -353,18 +367,15 @@ sub build_query {
     # query.
     my $acn_joined = 0;
     # Join to the acn table as needed for the library option.
-    if ($Marque::config->option_value('library')) {
+    if (@{$self->{libs}}) {
         $acn_joined = 1;
         $from .= <<ACN_JOIN;
 
 join $acnTable on $acnTable.record = $breTable.id
 and $acnTable.deleted = 'f'
-join (select id from actor.org_unit where shortname in (
+and $acnTable.owning_lib in (
 ACN_JOIN
-
-        $from .= join(',',
-                      map {"'$_'"} @{$Marque::config->option_value('library')});
-        $from .= ") aou on $acnTable.owning_lib = aou.id";
+        $from .= join(',', @{$self->{libs}}) . ")";
     }
 
     if ($Marque::config->option_value('items')) {
@@ -417,7 +428,7 @@ sub next {
                                                 $Marque::config->option_value('format'));
             };
             if ($@) {
-                print STDERR "Error in authority record " . $bre->id() . "\n";
+                print STDERR "Error in bibliograpic record " . $bre->id() . "\n";
                 print STDERR "$@\n";
                 import MARC::File::XML; # Reset SAX Parser.
                 return $self->next();
@@ -536,11 +547,9 @@ sub acns_for_bre {
         my $query = "select " . join(',', $self->{acnClass}->real_fields());
         $query .= "\nfrom " . $self->{acnClass}->Table();
         $query .= "\nwhere record = ? and deleted = 'f'";
-        if ($Marque::config->option_value('library')) {
+        if (@{$self->{libs}}) {
             $query .= "\nand owning_lib in (";
-            $query .= "select id from actor.org_unit where shortname in (";
-            $query .= join(',', map {"'$_'"} @{$Marque::config->option_value('library')});
-            $query .= "))";
+            $query .= join(',', @{$self->{libs}}) . ")";
         }
         $self->{acnHandle} = $self->{handle}->prepare($query);
     }