Description
Tomás Cohen Arazi (tcohen)
2025-08-01 14:31:35 UTC
Created attachment 185034 [details] [review] 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 Created attachment 185035 [details] [review] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Created attachment 185078 [details] [review] 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> Created attachment 185079 [details] [review] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed Off There's not enough info to do manual tests, but the unit tests work and the code change itself looks good. Created attachment 185737 [details] [review] 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> Created attachment 185738 [details] [review] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> While I think this is comparatively hard to exploit (at least in Patron import, where I tested it), Tomás patches address it correctly and the test for detection is good! Definitely a nice improvement. Would you also write that guideline, Tomás? Is this required for 22.11.x? The following files don't exist in 22.11.x: * Koha/BackgroundJob/ImportKBARTFile.pm * Koha/ERM/EUsage/CounterFile.pm But this one does: * Koha/Patrons/Import.pm Created attachment 185783 [details] [review] [22.11.x] 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 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> Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz> Created attachment 185784 [details] [review] [22.11.x] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 1 file without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 185785 [details] [review] [22.11.x] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 1 file without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Signed-off-by: wainuiwitikapark <wainuiwitikapark@catalyst.net.nz> (In reply to Wainui Witika-Park from comment #9) > Is this required for 22.11.x? > > The following files don't exist in 22.11.x: > > * Koha/BackgroundJob/ImportKBARTFile.pm > * Koha/ERM/EUsage/CounterFile.pm > > But this one does: > > * Koha/Patrons/Import.pm Ok I just rebased the patches to take out the first 2 files and only keep in changes for Koha/Patrons/Import.pm (In reply to Wainui Witika-Park from comment #13) > (In reply to Wainui Witika-Park from comment #9) > > Is this required for 22.11.x? > > > > The following files don't exist in 22.11.x: > > > > * Koha/BackgroundJob/ImportKBARTFile.pm > > * Koha/ERM/EUsage/CounterFile.pm > > > > But this one does: > > > > * Koha/Patrons/Import.pm > > Ok I just rebased the patches to take out the first 2 files and only keep in > changes for Koha/Patrons/Import.pm Applied the 22.11.x patches to 22.11.x-security for 22.11.30 This doesn't apply to 24.05.x. Merge conflicts. Please resolve. Created attachment 185796 [details] [review] [24.05.x] 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> Created attachment 185797 [details] [review] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Damn, something went wrong when attaching. Fixing.. Created attachment 185798 [details] [review] 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> Created attachment 185799 [details] [review] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Created attachment 185800 [details] [review] [24.05.x] 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> Created attachment 185801 [details] [review] [24.05.x] Bug 40579: Fix CSV formula injection vulnerabilities This patch addresses CSV formula injection vulnerabilities by ensuring all Text::CSV instances use formula protection. Changes made: * Add formula => 'empty' to all Text::CSV constructors in: - Koha::Patrons::Import - Koha::BackgroundJob::ImportKBARTFile - Koha::ERM::EUsage::CounterFile The 'empty' formula mode causes potentially dangerous formulas (starting with =, +, -, @) to be treated as empty strings rather than executed, preventing CSV injection attacks while maintaining data integrity. Security impact: * Prevents execution of malicious formulas in CSV imports * Protects against data exfiltration via CSV formula injection * Maintains backward compatibility for legitimate CSV data To test: 1. Apply the previous patch (author test) 2. Run: $ ktd --shell k$ prove xt/author/Text_CSV_Various.t => FAIL: Should show 3 files without formula protection 3. Apply this patch 4. Repeat 2 => SUCCESS: All tests should now pass 5. Try importing CSV with formula content (=cmd|"/c calc",test,data) => SUCCESS: Formula content is rejected/emptied, not executed 6. Import normal CSV data => SUCCESS: Normal data imports correctly 7. Sign off :-D Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Paul Derscheid <paul.derscheid@lmscloud.de> Alright, fixed and added rebased versions for 24.05.x as well. Sorry for the mess. Applied to 25.05.x-security for 25.05.03. |