From 19df1379df641b33aa6d1a032b7e548d8c91e1ae Mon Sep 17 00:00:00 2001
From: Kyle M Hall <kyle@bywatersolutions.com>
Date: Thu, 27 Jul 2023 12:30:54 -0400
Subject: [PATCH] Bug 33339: Prevent Formula Injection (CSV Injection) in CSV
 files

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 "none" which prevents those
formula from being exported.

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>
---
 C4/Acquisition.pm                      | 10 +++++++++-
 C4/ImportExportFramework.pm            |  2 +-
 C4/Labels/Label.pm                     |  2 +-
 C4/Record.pm                           |  2 +-
 Koha/Patrons/Import.pm                 |  2 +-
 admin/aqplan.pl                        |  4 +++-
 circ/overdue.pl                        |  2 +-
 labels/label-create-csv.pl             |  2 +-
 misc/cronjobs/overdue_notices.pl       |  2 +-
 misc/cronjobs/runreport.pl             | 15 +++++++++------
 misc/export_borrowers.pl               |  2 +-
 misc/migration_tools/import_lexile.pl  |  2 +-
 reports/cash_register_stats.pl         |  1 -
 reports/guided_reports.pl              |  2 +-
 reports/itemslost.pl                   |  3 +--
 serials/lateissues-export.pl           | 15 +++++++++------
 t/db_dependent/Record/marcrecord2csv.t |  2 +-
 tools/import_borrowers.pl              |  2 +-
 tools/inventory.pl                     |  6 +++---
 tools/viewlog.pl                       |  2 +-
 20 files changed, 47 insertions(+), 33 deletions(-)

diff --git a/C4/Acquisition.pm b/C4/Acquisition.pm
index 6824bd86371..296e9d32192 100644
--- a/C4/Acquisition.pm
+++ b/C4/Acquisition.pm
@@ -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 =~ /
diff --git a/C4/ImportExportFramework.pm b/C4/ImportExportFramework.pm
index 3d44d46ec39..d02ec43d574 100644
--- a/C4/ImportExportFramework.pm
+++ b/C4/ImportExportFramework.pm
@@ -965,7 +965,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;
diff --git a/C4/Labels/Label.pm b/C4/Labels/Label.pm
index 1d6ea20e8bc..702f5661694 100644
--- a/C4/Labels/Label.pm
+++ b/C4/Labels/Label.pm
@@ -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
diff --git a/C4/Record.pm b/C4/Record.pm
index 29f105a9f10..e4aac130dd1 100644
--- a/C4/Record.pm
+++ b/C4/Record.pm
@@ -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";
diff --git a/Koha/Patrons/Import.pm b/Koha/Patrons/Import.pm
index 5c8dc6b8763..f4b5c2538e7 100644
--- a/Koha/Patrons/Import.pm
+++ b/Koha/Patrons/Import.pm
@@ -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 =
diff --git a/admin/aqplan.pl b/admin/aqplan.pl
index 676b247b1f9..b9b41215002 100755
--- a/admin/aqplan.pl
+++ b/admin/aqplan.pl
@@ -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(
diff --git a/circ/overdue.pl b/circ/overdue.pl
index dacc8a9301c..7af624600b5 100755
--- a/circ/overdue.pl
+++ b/circ/overdue.pl
@@ -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();
 
diff --git a/labels/label-create-csv.pl b/labels/label-create-csv.pl
index a6c6f943ee5..fe4600a80e0 100755
--- a/labels/label-create-csv.pl
+++ b/labels/label-create-csv.pl
@@ -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(
diff --git a/misc/cronjobs/overdue_notices.pl b/misc/cronjobs/overdue_notices.pl
index 6f78b2838a1..5e243616991 100755
--- a/misc/cronjobs/overdue_notices.pl
+++ b/misc/cronjobs/overdue_notices.pl
@@ -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 {
diff --git a/misc/cronjobs/runreport.pl b/misc/cronjobs/runreport.pl
index ee807d53cac..f1a51d6975a 100755
--- a/misc/cronjobs/runreport.pl
+++ b/misc/cronjobs/runreport.pl
@@ -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} };
diff --git a/misc/export_borrowers.pl b/misc/export_borrowers.pl
index 7cde608ecef..b6a19d4dd84 100755
--- a/misc/export_borrowers.pl
+++ b/misc/export_borrowers.pl
@@ -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
diff --git a/misc/migration_tools/import_lexile.pl b/misc/migration_tools/import_lexile.pl
index 0d506ccc0f4..c30ec0e4681 100755
--- a/misc/migration_tools/import_lexile.pl
+++ b/misc/migration_tools/import_lexile.pl
@@ -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: $!";
diff --git a/reports/cash_register_stats.pl b/reports/cash_register_stats.pl
index f0e3161ac3f..2a7de38101b 100755
--- a/reports/cash_register_stats.pl
+++ b/reports/cash_register_stats.pl
@@ -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;
diff --git a/reports/guided_reports.pl b/reports/guided_reports.pl
index 49687d39886..062a480c9e0 100755
--- a/reports/guided_reports.pl
+++ b/reports/guided_reports.pl
@@ -646,7 +646,7 @@ elsif ($op eq 'export'){
             if ( $format eq 'csv' ) {
                 my $delimiter = C4::Context->csv_delimiter;
                 $type = 'application/csv';
-                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";
diff --git a/reports/itemslost.pl b/reports/itemslost.pl
index 9f1a990f47c..80c84bb633d 100755
--- a/reports/itemslost.pl
+++ b/reports/itemslost.pl
@@ -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";
diff --git a/serials/lateissues-export.pl b/serials/lateissues-export.pl
index dff0e5a7acd..12130d44347 100755
--- a/serials/lateissues-export.pl
+++ b/serials/lateissues-export.pl
@@ -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 );
diff --git a/t/db_dependent/Record/marcrecord2csv.t b/t/db_dependent/Record/marcrecord2csv.t
index 53d80918abc..bf3df2101fd 100755
--- a/t/db_dependent/Record/marcrecord2csv.t
+++ b/t/db_dependent/Record/marcrecord2csv.t
@@ -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 );
diff --git a/tools/import_borrowers.pl b/tools/import_borrowers.pl
index 048d053d47c..25686c205e6 100755
--- a/tools/import_borrowers.pl
+++ b/tools/import_borrowers.pl
@@ -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',
diff --git a/tools/inventory.pl b/tools/inventory.pl
index e38d7c4b651..1c941835964 100755
--- a/tools/inventory.pl
+++ b/tools/inventory.pl
@@ -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',
diff --git a/tools/viewlog.pl b/tools/viewlog.pl
index 91cd65df252..b1c27b0c526 100755
--- a/tools/viewlog.pl
+++ b/tools/viewlog.pl
@@ -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
-- 
2.39.2