Bug 11666: remove SQL as an option for MARC framework exports and imports

The SQL option for MARC framework imports was subject to a bug whereby
somebody could use it to gain access to arbitrary information in the
database by uploading an SQL file containing unexpected statements.

As it is difficult to securely sanitize SQL, this patch removes the
option to use SQL as an import or export format.

To test:

[1] Verify that SQL no longer appears as an import or export option
    for the MARC frameworks.
[2] Verify that exports and imports in CSV, Excel XML, and ODS formats
    still work.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>
Signed-off-by: Tomas Cohen Arazi <tomascohen@gmail.com>
Works as advertised. The UI doesn't offer exporting/importing in the SQL format.
Crafting the URL to export SQL fallbacks to a spreadsheet format (ODS).

Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Works as described, passes all tests and QA script.

Signed-off-by: Galen Charlton <gmc@esilibrary.com>

diff --git a/C4/ImportExportFramework.pm b/C4/ImportExportFramework.pm
index f85101d..8769a79 100644 (file)
--- a/C4/ImportExportFramework.pm
+++ b/C4/ImportExportFramework.pm
@@ -230,8 +230,8 @@ sub ExportFramework
             }
         }
 
-        if (_export_table('marc_tag_structure', $dbh, ($mode eq 'csv' || $mode eq 'sql')?$xmlStrRef:$dom, ($mode eq 'ods')?$elementSS:$root, $frameworkcode, $mode)) {
-            if (_export_table('marc_subfield_structure', $dbh, ($mode eq 'csv' || $mode eq 'sql')?$xmlStrRef:$dom, ($mode eq 'ods')?$elementSS:$root, $frameworkcode, $mode)) {
+        if (_export_table('marc_tag_structure', $dbh, ($mode eq 'csv')?$xmlStrRef:$dom, ($mode eq 'ods')?$elementSS:$root, $frameworkcode, $mode)) {
+            if (_export_table('marc_subfield_structure', $dbh, ($mode eq 'csv')?$xmlStrRef:$dom, ($mode eq 'ods')?$elementSS:$root, $frameworkcode, $mode)) {
                 $$xmlStrRef = $dom->toString(1) if ($mode eq 'ods' || $mode eq 'excel');
                 return 1;
             }
@@ -249,8 +249,6 @@ sub _export_table
     my ($table, $dbh, $dom, $root, $frameworkcode, $mode) = @_;
     if ($mode eq 'csv') {
         _export_table_csv($table, $dbh, $dom, $root, $frameworkcode);
-    } elsif ($mode eq 'sql') {
-        _export_table_sql($table, $dbh, $dom, $root, $frameworkcode);
     } elsif ($mode eq 'ods') {
         _export_table_ods($table, $dbh, $dom, $root, $frameworkcode);
     } else {
@@ -258,46 +256,6 @@ sub _export_table
     }
 }
 
-
-# Export the mysql table to an sql file
-sub _export_table_sql
-{
-    my ($table, $dbh, $strSQL, $root, $frameworkcode) = @_;
-
-    eval {
-        # First row with the name of the columns
-        my $query = 'SHOW COLUMNS FROM ' . $table;
-        my $sth = $dbh->prepare($query);
-        $sth->execute();
-        my @fields = ();
-        while (my $hashRef = $sth->fetchrow_hashref) {
-            push @fields, $hashRef->{Field};
-        }
-        my $fields = join(',', @fields);
-        $$strSQL .= 'DELETE FROM ' . $table . ' WHERE frameworkcode=' . $dbh->quote($frameworkcode) . ';';
-        $$strSQL .= chr(10);
-        # Populate rows with the data from mysql
-        $query = 'SELECT * FROM ' . $table . ' WHERE frameworkcode=?';
-        $sth = $dbh->prepare($query);
-        $sth->execute($frameworkcode);
-        while (my $hashRef = $sth->fetchrow_hashref) {
-            $$strSQL .= 'INSERT INTO ' . $table . ' (' . $fields . ') VALUES (';
-            for (@fields) {
-                $$strSQL .= $dbh->quote($hashRef->{$_}) . ',';
-            }
-            chop $$strSQL;
-            $$strSQL .= ');' . chr(10);
-        }
-        $$strSQL .= chr(10) . chr(10);
-    };
-    if ($@) {
-        $debug and warn "Error _export_table_sql $@\n";
-        return 0;
-    }
-    return 1;
-}#_export_table_sql
-
-
 # Export the mysql table to an csv file
 sub _export_table_csv
 {
@@ -677,7 +635,7 @@ sub ImportFramework
     my $dbh = C4::Context->dbh;
     if (-r $filename && $dbh) {
         my $extension = '';
-        if ($filename =~ /\.(csv|ods|xml|sql)$/i) {
+        if ($filename =~ /\.(csv|ods|xml)$/i) {
             $extension = lc($1);
         } else {
             unlink ($filename) if ($deleteFilename); # remove temporary file
@@ -701,20 +659,14 @@ sub ImportFramework
                     open($dom, '<', $filename);
                 }
                 if ($dom) {
-                    # For sql we execute the line
-                    if ($extension eq 'sql') {
-                        _parseSQLLine($dbh, $dom, $frameworkcode);
-                        $ok = 0;
-                    } else {
-                        # Process both tables
-                        my $numDeleted = 0;
-                        my $numDeletedAux = 0;
-                        if (($numDeletedAux = _import_table($dbh, 'marc_tag_structure', $frameworkcode, $dom, ['frameworkcode', 'tagfield'], $extension)) >= 0) {
+                    # Process both tables
+                    my $numDeleted = 0;
+                    my $numDeletedAux = 0;
+                    if (($numDeletedAux = _import_table($dbh, 'marc_tag_structure', $frameworkcode, $dom, ['frameworkcode', 'tagfield'], $extension)) >= 0) {
+                        $numDeleted += $numDeletedAux if ($numDeletedAux > 0);
+                        if (($numDeletedAux = _import_table($dbh, 'marc_subfield_structure', $frameworkcode, $dom, ['frameworkcode', 'tagfield', 'tagsubfield'], $extension)) >= 0) {
                             $numDeleted += $numDeletedAux if ($numDeletedAux > 0);
-                            if (($numDeletedAux = _import_table($dbh, 'marc_subfield_structure', $frameworkcode, $dom, ['frameworkcode', 'tagfield', 'tagsubfield'], $extension)) >= 0) {
-                                $numDeleted += $numDeletedAux if ($numDeletedAux > 0);
-                                $ok = ($numDeleted > 0)?$numDeleted:0;
-                            }
+                            $ok = ($numDeleted > 0)?$numDeleted:0;
                         }
                     }
                 } else {
@@ -724,7 +676,7 @@ sub ImportFramework
             if ($@) {
                 $debug and warn "Error ImportFramework $@\n";
             } else {
-                if ($extension eq 'sql' || $extension eq 'csv') {
+                if ($extension eq 'csv') {
                     close($dom) if ($dom);
                 }
             }
@@ -746,155 +698,6 @@ sub ImportFramework
     return $ok;
 }#ImportFramework
 
-
-# Parse the sql statement to see if the frameworkcode is correct
-# We're checking only the delete and insert SQL commands generated in the export process
-sub _parseSQLLine
-{
-    my ($dbh, $dom, $frameworkcode) = @_;
-
-    my $parser;
-    eval {
-        require SQL::Statement;
-        $parser = SQL::Parser->new('AnyData');
-        $parser->{RaiseError}=1;
-        $parser->{PrintError}=0;
-    };
-    my $literalEscape = (C4::Context->config("db_scheme") eq 'mysql')?'\\':'\'';
-    my $line;
-    my $numLines = 0;
-    while (<$dom>) {
-        s/[\r\n]+$//;
-        $line = $_;
-        # we don't want to execute any sql statement, only the ones dealing with frameworks
-        next unless ($line =~ /^\s*(?i:DELETE\s+FROM|INSERT\s+INTO)\s+(?:marc_tag_structure|marc_subfield_structure)/);
-        $numLines++;
-        # We check if the frameworkcode is the same, if not we change it
-        unless ($line =~ /'$frameworkcode'/) {
-            my $error = 0;
-            if ($parser) {
-                eval {
-                    $line = substr($line, 0 ,-1) if ($line =~ /;$/);
-                    my $stmt = SQL::Statement->new($line, $parser);
-                    my $where = $stmt->where();
-                    if ($where && $where->op() eq '=' && $line =~ /^\s*DELETE/) {
-                        $line =~ s/frameworkcode='.*?'/frameworkcode='$frameworkcode';/ unless ($_ =~ /frameworkcode='$frameworkcode'/);
-                    } else {
-                        my @arrFields;
-                        my @arrValues;
-                        my $table;
-                        # Due to lacking of backward compatibility
-                        if ($parser->VERSION < 1.30) {
-                            $table = lc($stmt->tables(0)->name());
-                            @arrFields = map{lc($_->name)} $stmt->columns;
-                            @arrValues = $stmt->row_values();
-                        } else {
-                            $table = $stmt->tables(0)->name();
-                            @arrValues = $stmt->row_values(0);
-                            my @aux = $stmt->column_defs();
-                            for (@{$aux[0]}) {
-                                push @arrFields, $_->{value};
-                            }
-                        }
-                        if (scalar(@arrFields) == scalar(@arrValues)) {
-                            my $j = 0;
-                            my $modified = 0;
-                            for (@arrFields) {
-                                if ($_ eq 'frameworkcode' && $arrValues[$j] ne $frameworkcode) {
-                                    $arrValues[$j] = $dbh->quote($frameworkcode);
-                                    $modified = 1;
-                                } else {
-                                    $arrValues[$j] = $dbh->quote($arrValues[$j]);
-                                }
-                                $j++;
-                            }
-                            $line = 'INSERT INTO ' . $table . ' (' . join(',', @arrFields) . ') VALUES (' . join(',', @arrValues) . ');' if ($modified);
-                        }
-                    }
-                };
-                $error = 1 if ($@);
-            } else {
-                $error = 1;
-            }
-            if ($error) {
-                $line .= ';' unless ($line =~ /;$/);
-                if ($line =~ /^\s*DELETE/) {
-                    $line =~ s/frameworkcode='.*?'/frameworkcode='$frameworkcode'/ unless ($_ =~ /frameworkcode='$frameworkcode'/);
-                } elsif ($line =~ /^\s*INSERT\s+INTO\s+(.*?)\s+\((.*?frameworkcode.*?)\)\s+VALUES\s+\((.+)\)\s*;\s*$/) {
-                    my $table = $1;
-                    my $fields = $2;
-                    my $values = $3;
-                    my @arrFields = split(/\s*,\s*/, $fields);
-                    my @arrValues;
-                    if ($values) {
-                        _parseSQLInsertValues($values, $literalEscape, \@arrValues);
-                    }
-                    if (scalar(@arrFields) == scalar(@arrValues)) {
-                        my $modified = 0;
-                        for (my $i=0; $i < @arrFields; $i++) {
-                            if ($arrFields[$i] eq 'frameworkcode' && $arrValues[$i]->{value} ne $frameworkcode) {
-                                $arrValues[$i]->{value} = $dbh->quote($frameworkcode);
-                                $modified = 1;
-                            } elsif ($arrValues[$i]->{literal}) {
-                                $arrValues[$i]->{value} = $dbh->quote($arrValues[$i]->{value});
-                            }
-                        }
-                        if ($modified) {
-                            $line = "INSERT INTO $table ($fields) VALUES (" . join(',', map {$_->{value}} @arrValues) . ');';
-                        }
-                    }
-                }
-            }
-        }
-        eval {
-            $dbh->do($line);
-        };
-    }
-}#_parseSQLLine
-
-
-# Simple sub to get the values from the insert sentence
-sub _parseSQLInsertValues
-{
-    my ($values, $literalEscape, $arrValues) = @_;
-
-    my ($posBegin, $posLiteral, $currentPos, $lengthValues, $currentChar);
-    $lengthValues = length($values);
-    $currentPos = 0;
-    while ($currentPos < $lengthValues) {
-        $currentChar = substr($values, $currentPos++, 1);
-        next if ($currentChar =~ /^\s$/);
-        next if ($posBegin && $currentChar !~ /^[,']$/);
-        unless ($posBegin) {
-            if ($currentChar eq '\'') {
-                $posBegin = $currentPos;
-                $posLiteral = $posBegin;
-            } else {
-                $posBegin = $currentPos -1;
-            }
-        } else {
-            if ($currentChar eq ',') {
-                unless ($posLiteral) {
-                    push @$arrValues, {literal => 0, value => substr($values, $posBegin, $currentPos -(1 + $posBegin))};
-                    $posBegin = undef;
-                }
-            } elsif ($currentChar eq '\'' && $posLiteral) {
-                next if ($literalEscape eq '\\' && substr($values, $currentPos -2, 1) eq $literalEscape);
-                if ($literalEscape eq '\'' && substr($values, $currentPos, 1) eq $literalEscape) {
-                    $currentPos++;
-                    next;
-                }
-                push @$arrValues, {literal => 1 , value => substr($values, $posBegin, $currentPos -( 1 + $posBegin))};
-                $currentPos++ if (substr($values, $currentPos, 1) eq ',');
-                $posBegin = undef;
-                $posLiteral = undef;
-            } # We shouldn't get to here if the sql sentence is correct
-        }
-   }
-   push @$arrValues, {literal => ($posLiteral)?1:0, value => substr($values, $posBegin, $currentPos - $posBegin)} if ($posBegin);
-}#_parseSQLInsertValues
-
-
 # Open (uncompress) ods file and return the content.xml file
 sub _openODS
 {

diff --git a/admin/import_export_framework.pl b/admin/import_export_framework.pl
index 555eec5..2a95c45 100755 (executable)
--- a/admin/import_export_framework.pl
+++ b/admin/import_export_framework.pl
@@ -58,10 +58,6 @@ if ($action eq 'export' && $input->request_method() eq 'GET') {
         # CSV file
         print $input->header(-type => 'application/vnd.ms-excel', -attachment => 'export_' . $frameworkcode . '.csv');
         print $strXml;
-    } elsif ($format eq 'sql') {
-        # SQL file
-        print $input->header(-type => 'text/plain', -attachment => 'export_' . $frameworkcode . '.sql');
-        print $strXml;
     } elsif ($format eq 'excel') {
         # Excel-xml file
         print $input->header(-type => 'application/excel', -attachment => 'export_' . $frameworkcode . '.xml');
@@ -79,7 +75,7 @@ if ($action eq 'export' && $input->request_method() eq 'GET') {
     my $fieldname = 'file_import_' . $frameworkcode;
     my $filename = $input->param($fieldname);
     # upload the input file
-    if ($filename && $filename =~ /\.(csv|ods|xml|sql)$/i) {
+    if ($filename && $filename =~ /\.(csv|ods|xml)$/i) {
         my $extension = $1;
         my $uploadFd = $input->upload($fieldname);
         if ($uploadFd && !$input->cgi_error) {

diff --git a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt
index 50faf31..e6ce9d8 100644 (file)
--- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt
+++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt
@@ -76,7 +76,7 @@ function Check(f) {
 
         $('input.input_import').change( function() {
             var filename = $(this).val();
-            if ( ! /(?:\.csv|\.sql|\.ods|\.xml)$/.test(filename)) {
+            if ( ! /(?:\.csv|\.ods|\.xml)$/.test(filename)) {
                 $(this).css("background-color","yellow");
                 alert(_("Please select an ods or xml file"));
                 $(this).val("");
@@ -90,7 +90,7 @@ function Check(f) {
         $('form.form_import').submit(function() {
             var id = $(this).attr('id');
             var obj = $('#' + id + ' input:file');
-            if (/(?:\.csv|\.sql|\.ods|\.xml)$/.test(obj.val())) {
+            if (/(?:\.csv|\.ods|\.xml)$/.test(obj.val())) {
                 if (confirm(_("Do you really want to import the framework fields and subfields? This will overwrite the current configuration. For safety reasons please use the export option to make a backup"))) {
                     var frameworkcode = $('#' + id + ' input:hidden[name=frameworkcode]').val();
                     $('#importing_' + frameworkcode).find("span").html(_("Importing to framework:")+"<strong>" + frameworkcode + "</strong>. " +_("Importing from file:")+"<i>" + obj.val().replace(new RegExp("^.+[/\\\\]"),"") + "</i>");
@@ -107,7 +107,7 @@ function Check(f) {
                     return false;
             }
             obj.css("background-color","yellow");
-            alert(_("Please select an spreadsheet (csv, ods, xml) or sql file"));
+            alert(_("Please select an spreadsheet (csv, ods, xml) file"));
             obj.val("");
             obj.css("background-color","white");
             return false;
@@ -184,8 +184,8 @@ function Check(f) {
         <th>&nbsp;</th>
         <th>Edit</th>
         <th>Delete</th>
-        <th title="Export framework structure (fields, subfields) to a spreadsheet file (.csv, .xml, .ods) or SQL file">Export</th>
-        <th title="Import framework structure (fields, subfields) from a spreadsheet file (.csv, .xml, .ods) or SQL file">Import</th>
+        <th title="Export framework structure (fields, subfields) to a spreadsheet file (.csv, .xml, .ods)">Export</th>
+        <th title="Import framework structure (fields, subfields) from a spreadsheet file (.csv, .xml, .ods)">Import</th>
     </tr>
     <tr>
         <td>&nbsp;</td>
@@ -210,7 +210,6 @@ function Check(f) {
                             <p><label for="csv_type_export_[% frameworkcode %]"><input type="radio" name="type_export_[% frameworkcode %]" value="csv" id="csv_type_export_[% frameworkcode %]" checked="checked" /> Export to CSV spreadsheet</label></p>
                             <p><label for="xml_type_export_[% frameworkcode %]"><input type="radio" name="type_export_[% frameworkcode %]" value="excel" id="xml_type_export_[% frameworkcode %]" /> Export to Excel with XML format, compatible with OpenOffice/LibreOffice as well</label></p>
                             <p><label for="ods_type_export_[% frameworkcode %]"><input type="radio" name="type_export_[% frameworkcode %]" value="ods" id="ods_type_export_[% frameworkcode %]" /> Export to OpenDocument spreadsheet format</label></p>
-                            <p><label for="sql_type_export_[% frameworkcode %]"><input type="radio" name="type_export_[% frameworkcode %]" value="sql" id="sql_type_export_[% frameworkcode %]" /> Export to SQL</label></p>
 
                         </fieldset>
                     </div>
@@ -230,7 +229,7 @@ function Check(f) {
             <div class="modal hide" id="importModal_[% frameworkcode %][% loop.count %]" tabindex="-1" role="dialog" aria-labelledby="importLabelexportModal_[% frameworkcode %][% loop.count %]" aria-hidden="true">
                 <div class="modal-header">
                     <button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">×</button>
-                    <h3 id="importLabelexportModal_[% frameworkcode %][% loop.count %]">Import default framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods) or SQL file</h3>
+                    <h3 id="importLabelexportModal_[% frameworkcode %][% loop.count %]">Import default framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods)</h3>
                 </div>
                 <form action="/cgi-bin/koha/admin/import_export_framework.pl" name="form_i_[% frameworkcode %]" id="form_i_[% frameworkcode %]" method="post" enctype="multipart/form-data" class="form_import">
                     <div class="modal-body">
@@ -275,7 +274,6 @@ function Check(f) {
                                 <p><label for="csv_type_export_[% loo.frameworkcode %][% loop.count %]"><input type="radio" name="type_export_[% loo.frameworkcode %]" value="csv" id="csv_type_export_[% loo.frameworkcode %][% loop.count %]" checked="checked" /> Export to CSV spreadsheet</label></p>
                                 <p><label for="xml_type_export_[% loo.frameworkcode %][% loop.count %]"><input type="radio" name="type_export_[% loo.frameworkcode %]" value="excel" id="xml_type_export_[% loo.frameworkcode %][% loop.count %]" /> Export to Excel with XML format, compatible with OpenOffice/LibreOffice as well</label></p>
                                 <p><label for="ods_type_export_[% loo.frameworkcode %][% loop.count %]"><input type="radio" name="type_export_[% loo.frameworkcode %]" value="ods" id="ods_type_export_[% loo.frameworkcode %][% loop.count %]" /> Export to OpenDocument spreadsheet format</label></p>
-                                <p><label for="sql_type_export_[% loo.frameworkcode %][% loop.count %]"><input type="radio" name="type_export_[% loo.frameworkcode %]" value="sql" id="sql_type_export_[% loo.frameworkcode %][% loop.count %]" /> Export to SQL</label></p>
 
                             </fieldset>
                         </div>
@@ -294,7 +292,7 @@ function Check(f) {
                 <div class="modal hide" id="importModal_[% loo.frameworkcode %][% loop.count %]" tabindex="-1" role="dialog" aria-labelledby="importLabelexportModal_[% loo.frameworkcode %][% loop.count %]" aria-hidden="true">
                     <div class="modal-header">
                         <button type="button" class="closebtn" data-dismiss="modal" aria-hidden="true">×</button>
-                        <h3 id="importLabelexportModal_[% loo.frameworkcode %][% loop.count %]">Import [% loo.frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods) or SQL file</h3>
+                        <h3 id="importLabelexportModal_[% loo.frameworkcode %][% loop.count %]">Import [% loo.frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods)</h3>
                     </div>
                     <form action="/cgi-bin/koha/admin/import_export_framework.pl" name="form_i_[% loo.frameworkcode %]" id="form_i_[% loo.frameworkcode %]" method="post" enctype="multipart/form-data" class="form_import">
                         <div class="modal-body">