From 4c66b9da15b052ab2bcbf1fd966be8dd5d5ee270 Mon Sep 17 00:00:00 2001 From: Tomas Cohen Arazi Date: Fri, 1 Aug 2025 12:03:00 -0300 Subject: [PATCH] Bug 40579: Add author test for CSV formula injection protection This test ensures all Text::CSV usage in Koha includes proper formula injection protection to prevent CSV formula injection attacks. The test scans all Perl modules and verifies that Text::CSV constructors include the 'formula' parameter set to a safe mode ('empty', 'die', 'croak', 'diag', or numeric 1-5). To test: 1. Apply this patch 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Test should fail showing files without formula protection: - Koha/Patrons/Import.pm - Koha/BackgroundJob/ImportKBARTFile.pm - Koha/ERM/EUsage/CounterFile.pm 3. Apply the follow-up patch to fix the violations 4. Repeat 2 => SUCCESS: All tests should pass 5. Sign off :-D --- xt/author/Text_CSV_Various.t | 67 +++++++++++++++++++++++++++++++++++- 1 file changed, 66 insertions(+), 1 deletion(-) diff --git a/xt/author/Text_CSV_Various.t b/xt/author/Text_CSV_Various.t index 5521493ca99..1a0e9fde265 100755 --- a/xt/author/Text_CSV_Various.t +++ b/xt/author/Text_CSV_Various.t @@ -24,10 +24,11 @@ use Modern::Perl; use open OUT => ':encoding(UTF-8)', ':std'; use utf8; -use Test::More tests => 22; +use Test::More tests => 23; use Test::NoWarnings; use Text::CSV; use Text::CSV_XS; +use File::Find; sub pretty_line { my $max = 54; @@ -91,3 +92,67 @@ foreach my $line (@$lines) { } } } + +# Test for CSV formula injection protection +subtest 'CSV formula injection protection' => sub { + my @csv_files; + my @violations; + + # Find all Perl files that might use Text::CSV + find( + sub { + return unless -f $_ && /\.pm$/; + my $file = $File::Find::name; + + # Skip test files and this test file itself + return if $file =~ m{/t/} || $file =~ m{/xt/}; + + open my $fh, '<', $_ or return; + my $content = do { local $/; <$fh> }; + close $fh; + + # Look for actual Text::CSV usage (not just dependency declarations) + my @csv_usages = $content =~ /(?:use\s+Text::CSV|Text::CSV(?:_XS|::Encoded)?(?:\s*->|\s*\.\s*)new)/g; + return unless @csv_usages; + + # Skip if it's just dependency metadata (like in PerlModules.pm) + return if $content =~ /'Text::CSV[^']*'\s*=>\s*{[^}]*(?:required|min_ver|cur_ver)/; + + push @csv_files, $file; + + # Find all Text::CSV->new() calls and check each one + my @new_calls = $content =~ /(Text::CSV(?:_XS|::Encoded)?(?:\s*->|\s*\.\s*)new\s*\([^)]*\))/g; + my $has_unprotected = 0; + + for my $call (@new_calls) { + + # Check if this specific call has formula protection + unless ( $call =~ /formula\s*=>\s*['"](?:empty|die|croak|diag)['"]/ + || $call =~ /formula\s*=>\s*[1-5]/ ) + { + $has_unprotected = 1; + last; + } + } + + if ($has_unprotected) { + push @violations, $file; + } + }, + 'C4', 'Koha', + 'misc' + ); + + ok( scalar(@csv_files) > 0, "Found CSV usage in Koha modules" ); + + if (@violations) { + diag("Files using Text::CSV without formula protection:"); + diag(" $_") for @violations; + diag(""); + diag("CSV formula injection protection is required for security."); + diag("Add 'formula => \"empty\"' to Text::CSV constructor options."); + diag("Valid formula modes: 'empty' (recommended), 'die', 'croak', 'diag', or numeric 1-5"); + } + + is( scalar(@violations), 0, "All Text::CSV usage includes formula injection protection" ); +}; -- 2.50.1