Koha-community/Koha
15
7
Fork 0
Code Issues Releases Wiki Activity

Bug 33339: Prevent Formula Injection (CSV Injection) in CSV files

Browse source 
The system is vulnerable to Formula Injection attacks as the data
stored within the database and exported as CSV/Excel is not being
sanitized or validated against implanted formula payloads

This patch modifies all uses of Text::CSV and derived classes to pass
the "formula" parameter with value of "empty" which replaces formulas
by empty string.

Test Plan:
1) Apply this patch
2) For guided_reports.pl, attempt to export CSV where you've set a column to a formula somehow
   ( such as "=1+3" )
3) Export that CSV file
4) Note the formula has not been exported
5) Repeat this plan for the remaining scripts that export CSV files
   where users can define the outputted data

Signed-off-by: Magnus Enger <magnus@libriotech.no>
Fixed two conflicts. I have tested that this works as advertised on:
- Reports (Download > Comma separated text (.csv)) [Text::CSV::Encoded]
- Circulation > Overdues > Download file of all overdues [Text::CSV_XS]
- misc/export_borrowers.pl [Text::CSV]
This covers all modules used, and both GUI and command line.

Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>

Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl>
[EDIT] Change none to empty in the commit message ! None is the default,
doing nothing. Empty clears the formulas.
Signed-off-by: Katrin Fischer <katrin.fischer@bsz-bw.de>
This commit is contained in:
Kyle Hall 2023-07-27 12:30:54 -04:00 committed by Katrin Fischer
parent 5f5206d0e5
commit 6b96763992
Signed by: kfischer
GPG key ID: 0EF6E2C03357A834
20 changed files with 47 additions and 33 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
C4/Acquisition.pm
View file

@ -277,7 +277,15 @@ sub GetBasketAsCSV {
my $delimiter = $csv_profile->csv_separator;
$delimiter = "\t" if $delimiter eq "\\t";
my $csv = Text::CSV_XS->new({'quote_char'=>'"','escape_char'=>'"','sep_char'=>$delimiter,'binary'=>1});
my $csv = Text::CSV_XS->new(
{
quote_char => '"',
escape_char => '"',
sep_char => $delimiter,
binary => 1,
formula => "empty",
}
);
my $csv_profile_content = $csv_profile->content;
my ( @headers, @fields );
while ( $csv_profile_content =~ /

2
C4/ImportExportFramework.pm
View file

@ -966,7 +966,7 @@ sub _import_table_csv
shift @fieldsPK;
my $ok = 0;
my $pos = 0;
my $csv = Text::CSV_XS->new ({ binary => 1 });
my $csv = Text::CSV_XS->new( { binary => 1, formula => "empty" } );
while ( my $row = $csv->getline($dom) ) {
my @fields = @$row;
@arrData = @fields;

2
C4/Labels/Label.pm
View file

@ -115,7 +115,7 @@ sub _get_label_item {
sub _get_text_fields {
my $format_string = shift;
my $csv = Text::CSV_XS->new({allow_whitespace => 1});
my $csv = Text::CSV_XS->new( { allow_whitespace => 1, formula => "empty" } );
my $status = $csv->parse($format_string);
my @sorted_fields = map {{ 'code' => $_, desc => $_ }}
map { $_ && $_ eq 'callnumber' ? 'itemcallnumber' : $_ } # see bug 5653

2
C4/Record.pm
View file

@ -401,7 +401,7 @@ sub marc2csv {
my ($biblios, $id, $itemnumbers) = @_;
$itemnumbers ||= [];
my $output;
my $csv = Text::CSV::Encoded->new();
my $csv = Text::CSV::Encoded->new( { formula => "empty" } );
# Getting yaml file
my $configfile = "../tools/csv-profiles/$id.yaml";

2
Koha/Patrons/Import.pm
View file

@ -627,7 +627,7 @@ sub generate_patron_attributes {
push (@$feedback, { feedback => 1, name => 'attribute string', value => $string });
return [] unless $string; # Unit tests want the feedback, is it really needed?
my $csv = Text::CSV->new({binary => 1}); # binary needed for non-ASCII Unicode
my $csv = Text::CSV->new( { binary => 1, formula => "empty" } ); # binary needed for non-ASCII Unicode
my $ok = $csv->parse($string); # parse field again to get subfields!
my @list = $csv->fields();
my @patron_attributes =

4
admin/aqplan.pl
View file

@ -418,8 +418,10 @@ sub _print_to_csv {
binmode STDOUT, ':encoding(UTF-8)';
my $csv = Text::CSV_XS->new(
{ sep_char => $del,
{
sep_char => $del,
always_quote => 'TRUE',
formula => "empty",
}
);
print $input->header(

2
circ/overdue.pl
View file

@ -434,7 +434,7 @@ sub build_csv {
my @keys =
qw ( duedate title author borrowertitle firstname surname phone barcode email address address2 zipcode city country
branchcode datelastissued itemcallnumber biblionumber borrowernumber itemnum issuedate replacementprice itemnotes_nonpublic streetnumber streettype);
my $csv = Text::CSV_XS->new();
my $csv = Text::CSV_XS->new( { formula => "empty" } );
$csv->combine(@keys);
push @lines, $csv->string();

2
labels/label-create-csv.pl
View file

@ -73,7 +73,7 @@ else {
$items = $batch->get_attr('items');
}
my $csv = Text::CSV_XS->new();
my $csv = Text::CSV_XS->new( { formula => "empty" } );
foreach my $item (@$items) {
my $label = C4::Labels::Label->new(

2
misc/cronjobs/overdue_notices.pl
View file

@ -415,7 +415,7 @@ our $csv; # the Text::CSV_XS object
our $csv_fh; # the filehandle to the CSV file.
if ( defined $csvfilename ) {
my $sep_char = C4::Context->csv_delimiter;
$csv = Text::CSV_XS->new( { binary => 1 , sep_char => $sep_char } );
$csv = Text::CSV_XS->new( { binary => 1, sep_char => $sep_char, formula => "empty" } );
if ( $csvfilename eq '' ) {
$csv_fh = *STDOUT;
} else {

15
misc/cronjobs/runreport.pl
View file

@ -329,12 +329,15 @@ foreach my $report_id (@ARGV) {
}
$message = $cgi->table(join "", @rows);
} elsif ($format eq 'csv') {
my $csv = Text::CSV::Encoded->new({
encoding_out => 'utf8',
binary => 1,
quote_char => $quote,
sep_char => $separator,
});
my $csv = Text::CSV::Encoded->new(
{
encoding_out => 'utf8',
binary => 1,
quote_char => $quote,
sep_char => $separator,
formula => 'empty',
}
);
if ( $csv_header ) {
my @fields = map { decode( 'utf8', $_ ) } @{ $sth->{NAME} };

2
misc/export_borrowers.pl
View file

@ -94,7 +94,7 @@ unless ( $separator ) {
$separator = C4::Context->csv_delimiter;
}
my $csv = Text::CSV->new( { sep_char => $separator, binary => 1 } );
my $csv = Text::CSV->new( { sep_char => $separator, binary => 1, formula => 'empty' } );
# If the user did not specify any field to export, we assume they want them all
# We retrieve the first borrower informations to get field names

2
misc/migration_tools/import_lexile.pl
View file

@ -96,7 +96,7 @@ if ( $help || !$file || !$confirm ) {
my $schema = Koha::Database->new()->schema();
my $csv = Text::CSV->new( { binary => 1, sep_char => "\t" } )
my $csv = Text::CSV->new( { binary => 1, sep_char => "\t", formula => 'empty' } )
or die "Cannot use CSV: " . Text::CSV->error_diag();
open my $fh, "<:encoding(utf8)", $file or die "test.csv: $!";

1
reports/cash_register_stats.pl
View file

@ -23,7 +23,6 @@ use C4::Reports qw( GetDelimiterChoices );
use C4::Output qw( output_html_with_http_headers );
use DateTime;
use Koha::DateUtils qw( dt_from_string );
use Text::CSV::Encoded;
use List::Util qw( any );
use Koha::Account::CreditTypes;

2
reports/guided_reports.pl
View file

@ -649,7 +649,7 @@ elsif ($op eq 'export'){
# Add BOM for UTF-8 encoded CSV
$content .= "\xEF\xBB\xBF";
my $csv = Text::CSV::Encoded->new({ encoding_out => 'UTF-8', sep_char => $delimiter});
my $csv = Text::CSV::Encoded->new({ encoding_out => 'UTF-8', sep_char => $delimiter, formula => 'empty' });
$csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag();
if ( $csv->combine( header_cell_values($sth) ) ) {
$content .= $scrubber->scrub( Encode::decode( 'UTF-8', $csv->string() ) ) . "\n";

3
reports/itemslost.pl
View file

@ -28,7 +28,6 @@ This script displays lost items.
use Modern::Perl;
use CGI qw ( -utf8 );
use Text::CSV_XS;
use C4::Auth qw( get_template_and_user );
use C4::Output qw( output_html_with_http_headers );
use Text::CSV::Encoded;
@ -91,7 +90,7 @@ if ( $op eq 'cud-export' ) {
my $delimiter = $csv_profile->csv_separator;
$delimiter = "\t" if $delimiter eq "\\t";
my $csv = Text::CSV::Encoded->new({ encoding_out => 'UTF-8', sep_char => $delimiter});
my $csv = Text::CSV::Encoded->new( { encoding_out => 'UTF-8', sep_char => $delimiter, formula => 'empty' } );
$csv or die "Text::CSV::Encoded->new({binary => 1}) FAILED: " . Text::CSV::Encoded->error_diag();
$csv->combine(@headers);
my $content .= Encode::decode('UTF-8', $csv->string()) . "\n";

15
serials/lateissues-export.pl
View file

@ -46,12 +46,15 @@ die "There is no valid csv profile given" unless $csv_profile;
my $delimiter = $csv_profile->csv_separator;
$delimiter = "\t" if $delimiter eq "\\t";
my $csv = Text::CSV_XS->new({
'quote_char' => '"',
'escape_char' => '"',
'sep_char' => $delimiter,
'binary' => 1
});
my $csv = Text::CSV_XS->new(
{
quote_char => '"',
escape_char => '"',
sep_char => $delimiter,
binary => 1,
formula => 'empty',
}
);
my $content = $csv_profile->content;
my ( @headers, @fields );

2
t/db_dependent/Record/marcrecord2csv.t
View file

@ -30,7 +30,7 @@ my ( $biblionumber, $biblioitemnumber ) = AddBiblio( $record, $frameworkcode );
my $csv_content = q(Title=245$a|Author=245$c|Subject=650$a);
my $csv_profile_id_1 = insert_csv_profile({ csv_content => $csv_content });
my $csv = Text::CSV::Encoded->new();
my $csv = Text::CSV::Encoded->new( { formula => 'empty' } );
# Test bad biblionumber case
my $csv_output = C4::Record::marcrecord2csv( -1, $csv_profile_id_1, 1, $csv );

2
tools/import_borrowers.pl
View file

@ -78,7 +78,7 @@ $template->param( categories => \@patron_categories );
$template->param( borrower_fields => Koha::Database::Columns->columns->{borrowers} );
if ( $input->param('sample') ) {
our $csv = Text::CSV->new( { binary => 1 } ); # binary needed for non-ASCII Unicode
our $csv = Text::CSV->new( { binary => 1, formula => 'empty' } ); # binary needed for non-ASCII Unicode
print $input->header(
-type => 'application/vnd.sun.xml.calc', # 'application/vnd.ms-excel' ?
-attachment => 'patron_import.csv',

6
tools/inventory.pl
View file

@ -372,9 +372,9 @@ $template->param(
# Export to csv
if (defined $input->param('CSVexport') && $input->param('CSVexport') eq 'on'){
eval {use Text::CSV ();};
my $csv = Text::CSV->new or
die Text::CSV->error_diag ();
eval { use Text::CSV (); };
my $csv = Text::CSV->new( { formula => 'empty' } )
or die Text::CSV->error_diag();
binmode STDOUT, ":encoding(UTF-8)";
print $input->header(
-type => 'text/csv',

2
tools/viewlog.pl
View file

@ -234,7 +234,7 @@ if ($do_it) {
my $content = q{};
if (@data) {
my $delimiter = C4::Context->csv_delimiter;
my $csv = Text::CSV::Encoded->new( { encoding_out => 'utf8', sep_char => $delimiter } );
my $csv = Text::CSV::Encoded->new( { encoding_out => 'utf8', sep_char => $delimiter, formula => 'empty' } );
$csv or die "Text::CSV::Encoded->new FAILED: " . Text::CSV::Encoded->error_diag();
# First line with heading