From 50d8aa80fb62afc822a9093bbb2638a687e2487b Mon Sep 17 00:00:00 2001 From: Galen Charlton Date: Tue, 4 Feb 2014 23:03:08 +0000 Subject: [PATCH] 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 Signed-off-by: Tomas Cohen Arazi 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 Works as described, passes all tests and QA script. Signed-off-by: Galen Charlton Conflicts: koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt --- C4/ImportExportFramework.pm | 219 +----------------- admin/import_export_framework.pl | 6 +- .../prog/en/modules/admin/biblio_framework.tt | 24 +- 3 files changed, 23 insertions(+), 226 deletions(-) diff --git a/C4/ImportExportFramework.pm b/C4/ImportExportFramework.pm index 38b47c24c0..6fb2eee80e 100644 --- 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 { @@ -675,7 +633,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 @@ -699,20 +657,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 { @@ -722,7 +674,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); } } @@ -744,155 +696,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 555eec5e01..2a95c45014 100755 --- 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 b87ec20738..cbcc6a908d 100644 --- a/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt +++ b/koha-tmpl/intranet-tmpl/prog/en/modules/admin/biblio_framework.tt @@ -79,7 +79,7 @@ $(document).ready(function() { $('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(""); @@ -93,7 +93,7 @@ $(document).ready(function() { $('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:")+"" + frameworkcode + "
" +_("Importing from file:")+"" + obj.val().replace(new RegExp("^.+[/\\\\]"),"") + ""); @@ -109,7 +109,7 @@ $(document).ready(function() { 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; @@ -185,8 +185,8 @@ $(document).ready(function() {   Edit Delete - Export - Import + Export + Import   @@ -194,7 +194,7 @@ $(document).ready(function() { MARC structure     -
Export +
Export
@@ -204,7 +204,6 @@ $(document).ready(function() {
    • -
    • Cancel
    @@ -212,11 +211,11 @@ $(document).ready(function() {
    -
    Import +
    Import
    - Import [% frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods) or SQL file + Import [% frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods)
      @@ -241,7 +240,7 @@ $(document).ready(function() { Edit Delete -
      Export +
      Export
      @@ -251,7 +250,6 @@ $(document).ready(function() {
      4. -
    Cancel
    @@ -259,11 +257,11 @@ $(document).ready(function() {
    -
    Import +
    Import
    - Import [% frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods) or SQL file + Import [% frameworkcode %] framework structure (fields and subfields) from a spreadsheet file (.csv, .xml, .ods)
      -- 2.39.5