From 41e17032d6bc70ceea08c73eb2d8350cb99aa57f 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 (cherry picked from commit 94e349ff6ce4a1abb313102decc12429d02dfb4b) Signed-off-by: Tomas Cohen Arazi There were conflicts on the template. The modified strings wont get translated but in as it is an administrative feature that not everyone uses on a daily basis I think it wont hurt. And will get fixed in a couple of weeks anyway. --- 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 72873f2107..7b2979a8dc 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 { @@ -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 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 aa269ce4d9..73522919a8 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 @@ -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:")+"" + frameworkcode + "
" +_("Importing from file:")+"" + obj.val().replace(new RegExp("^.+[/\\\\]"),"") + ""); @@ -106,7 +106,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)")); obj.val(""); obj.css("background-color","white"); return false; @@ -183,8 +183,8 @@ function Check(f) {   Edit Delete - Export - Import + Export + Import   @@ -192,7 +192,7 @@ function Check(f) { MARC structure     -
Export +
Export
@@ -202,7 +202,6 @@ function Check(f) {
    • -
    • Cancel
    @@ -210,11 +209,11 @@ function Check(f) {
    -
    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)
      @@ -239,7 +238,7 @@ function Check(f) { Edit Delete -
      Export +
      Export
      @@ -249,7 +248,6 @@ function Check(f) {
      4. -
    Cancel
    @@ -257,11 +255,11 @@ function Check(f) {
    -
    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