Bugzilla – Attachment 185796 Details for
Bug 40579
CSV formula injection protection
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Help
|
New Account
|
Log In
[x]
|
Forgot Password
Login:
[x]
[patch]
[24.05.x] Bug 40579: Add author test for CSV formula injection protection
Bug-40579-Add-author-test-for-CSV-formula-injectio.patch (text/plain), 3.92 KB, created by
Paul Derscheid
on 2025-08-26 15:59:31 UTC
(
hide
)
Description:
[24.05.x] Bug 40579: Add author test for CSV formula injection protection
Filename:
MIME Type:
Creator:
Paul Derscheid
Created:
2025-08-26 15:59:31 UTC
Size:
3.92 KB
patch
obsolete
>From f7dbc75fdb0db115c13fbcc79a509c02e932ce89 Mon Sep 17 00:00:00 2001 >From: Tomas Cohen Arazi <tomascohen@theke.io> >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 > >Signed-off-by: David Cook <dcook@prosentient.com.au> >Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> >--- > 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 539234aefba..c5f65690bc6 100755 >--- a/xt/author/Text_CSV_Various.t >+++ b/xt/author/Text_CSV_Various.t >@@ -24,9 +24,10 @@ use Modern::Perl; > use open OUT=>':encoding(UTF-8)', ':std'; > use utf8; > >-use Test::More tests => 21; >+use Test::More tests => 22; > use Text::CSV; > use Text::CSV_XS; >+use File::Find; > > sub pretty_line { > my $max = 54; >@@ -82,3 +83,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.51.0
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Actions:
View
|
Diff
|
Splinter Review
Attachments on
bug 40579
:
185034
|
185035
|
185078
|
185079
|
185737
|
185738
|
185783
|
185784
|
185785
|
185796
|
185797
|
185798
|
185799
|
185800
|
185801