See "Bug 33339 - Formula Injection (CSV Injection) in Export Functionality" for background. DataTables can export CSV from tables, and the resulting CSV can include malicious formulas.
In bug 33339, comment 3 David Cook said: > (On a side note, locally we've actually removed the data export options shown in the "PoC screenshot" due to the ease of downloading borrower personal information. At some point, I might post a patch to remove those buttons...) Could you share it here, as a first step? I have found that commenting out this code in /home/magnus/code/kohaclone/koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc removes some of the CSV and Excel options from tables, but not from all of them: 65 var export_buttons = [ 66 // { 67 // extend: 'csvHtml5', 68 // text: _("CSV"), 69 // exportOptions: { 70 // columns: exportColumns, 71 // rows: exportRows, 72 // format: export_format 73 // }, 74 // }, 75 { 76 extend: 'copyHtml5', 77 text: _("Copy"), 78 exportOptions: { 79 columns: exportColumns, 80 rows: exportRows, 81 format: export_format 82 }, 83 }, 84 { 85 extend: 'print', 86 text: _("Print"), 87 exportOptions: { 88 columns: exportColumns, 89 rows: exportRows, 90 format: export_format 91 }, 92 } 93 ]; 94 95 [% IF Koha.Preference("CurrencyFormat") != 'FR' %] 96 // export_buttons.unshift ( 97 // { 98 // extend: 'excelHtml5', 99 // text: _("Excel"), 100 // exportOptions: { 101 // columns: exportColumns, 102 // rows: exportRows, 103 // format: export_format 104 // }, 105 // } 106 // ); 107 [% ELSE %] 108 // export_buttons.unshift ( 109 // { 110 // extend: 'excelHtml5', 111 // text: _("Excel"), 112 // exportOptions: { 113 // columns: exportColumns, 114 // rows: exportRows, 115 // format: export_numeric 116 // }, 117 // } 118 // ); 119 [% END %] Excel and CSV are still options on the item table on record detail view (e.g. /cgi-bin/koha/catalogue/detail.pl?biblionumber=360).
(In reply to Magnus Enger from comment #1) > Excel and CSV are still options on the item table on record detail view > (e.g. /cgi-bin/koha/catalogue/detail.pl?biblionumber=360). DTs using the Koha REST API are using kohaTable that is defined in koha-tmpl/intranet-tmpl/prog/js/datatables.js Same at the OPAC you will have koha-tmpl/opac-tmpl/bootstrap/en/includes/columns_settings.inc and koha-tmpl/opac-tmpl/bootstrap/js/datatables.js
(In reply to Magnus Enger from comment #1) > In bug 33339, comment 3 David Cook said: > > > (On a side note, locally we've actually removed the data export options shown in the "PoC screenshot" due to the ease of downloading borrower personal information. At some point, I might post a patch to remove those buttons...) > > Could you share it here, as a first step? It looks like I already did it as bug 33377. I'll add you on that one.
(In reply to David Cook from comment #3) > It looks like I already did it as bug 33377. I'll add you on that one. Cool, thanks! But that is a fix just for that one table. What is the change we would like to see upstream, in a nutshell? Any cell that would start with an equals sign (=) should be left empty?
(In reply to Magnus Enger from comment #4) > (In reply to David Cook from comment #3) > > It looks like I already did it as bug 33377. I'll add you on that one. > > Cool, thanks! But that is a fix just for that one table. > > What is the change we would like to see upstream, in a nutshell? Any cell > that would start with an equals sign (=) should be left empty? So bug 33377 isn't about formula injection. It's just about preventing the export of patron data via that particular UI. (Other places that patron data get exported have other protections locally.) For bug 37727 I think we'd need to actually fix DataTables. I remember writing something about this previously on a different bug...
Note: A CVE was opened for this on February 12 2024: https://app.opencve.io/cve/CVE-2024-24337
Ah right, I think the problem isn't with "DataTables" itself but rather with the "Buttons" extension.
./koha-tmpl/intranet-tmpl/lib/datatables/datatables.js comes with a rebuilder URL but since DataTables 1.x is no longer supported and DataTables 2.x is the new hotness, that rebuilder only works with 1.x. However, this should work: https://datatables.net/legacy/v1/download/#dt/jszip-3.10.1/pdfmake-0.2.7/dt-1.13.6/b-2.4.2/b-colvis-2.4.2/b-html5-2.4.2/b-print-2.4.2/fh-3.4.0/r-2.5.0 Unfortunately, it looks like we're already using the newest version of everything except DataTables which is now 1.13.11 and we're on 1.13.6.
This is one of the conversations about this topic but I think there's another one... https://datatables.net/forums/discussion/57856/excel-command-injection
I've created a new discussion on the DataTables forum for this with lots of information: https://datatables.net/forums/discussion/80069/csv-formula-injection-vulnerability-in-buttons-extension In the meantime, I'm going to look to see if there's any workarounds that we can put in place in the short-term...
(In reply to Magnus Enger from comment #1) > I have found that commenting out this code in > /home/magnus/code/kohaclone/koha-tmpl/intranet-tmpl/prog/en/includes/ > columns_settings.inc removes some of the CSV and Excel options from tables, > but not from all of them: Oh man... it looks like there's duplicated code in koha-tmpl/intranet-tmpl/prog/js/datatables.js as well as in ./koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc I'm looking at implementing a workaround while we wait on upstream, and this... was driving me crazy.
Going to have a late lunch then keep working on a hopefully robust workaround. I've got a PoC in progress that fixes /cgi-bin/koha/members/member.pl at least...
Ok, so the Buttons extension already auto-escapes double quotes in the cell value, so that's awesome: https://github.com/DataTables/Buttons/blob/master/js/buttons.html5.js#L274 So the workaround I've got should do the trick. I've shared it and some info with upstream, so we'll see what they say.
Ugh koha-tmpl/intranet-tmpl/prog/en/includes/columns_settings.inc has some additional weirdness with the "export_numeric" variable, so France needs some extra handling...
Created attachment 172834 [details] [review] Bug 37727: Prevent CSV Formula injection via DataTables This change prevents CSV Formula injection on DataTables exports by escaping formula with a single quote prefix as per OWASP recommendations. Test plan: 0. Apply patch 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl ?op=edit_form&destination=circ&borrowernumber=51 2. Add the following in a "Circulation note" =SUM(1+1) 3. Go to http://localhost:8081/cgi-bin/koha/members/member.pl ?quicksearch=1&circsearch=1&searchmember=koha 4. Click "Export" and choose "Excel" and "CSV" 5. Open those downloaded files in Excel 6. Note that the =SUM(1+1) function is prefixed with a single quote, and is not automatically executed
It's not a DRY solution, but it's easy to read and hopefully easy to understand, which I figured is more important in the short-term.
Created attachment 172942 [details] [review] Bug 37727: Prevent CSV Formula injection via DataTables This change prevents CSV Formula injection on DataTables exports by escaping formula with a single quote prefix as per OWASP recommendations. Test plan: 0. Apply patch 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl ?op=edit_form&destination=circ&borrowernumber=51 2. Add the following in a "Circulation note" =SUM(1+1) 3. Go to http://localhost:8081/cgi-bin/koha/members/member.pl ?quicksearch=1&circsearch=1&searchmember=koha 4. Click "Export" and choose "Excel" and "CSV" 5. Open those downloaded files in Excel 6. Note that the =SUM(1+1) function is prefixed with a single quote, and is not automatically executed Signed-off-by: Magnus Enger <magnus@libriotech.no> Works as advertised. The problematic "cell" is exported as "'=SUM(1+1)".
Created attachment 174535 [details] [review] Bug 37727: Prevent CSV Formula injection via DataTables This change prevents CSV Formula injection on DataTables exports by escaping formula with a single quote prefix as per OWASP recommendations. Test plan: 0. Apply patch 1. Go to http://localhost:8081/cgi-bin/koha/members/memberentry.pl ?op=edit_form&destination=circ&borrowernumber=51 2. Add the following in a "Circulation note" =SUM(1+1) 3. Go to http://localhost:8081/cgi-bin/koha/members/member.pl ?quicksearch=1&circsearch=1&searchmember=koha 4. Click "Export" and choose "Excel" and "CSV" 5. Open those downloaded files in Excel 6. Note that the =SUM(1+1) function is prefixed with a single quote, and is not automatically executed Signed-off-by: Magnus Enger <magnus@libriotech.no> Works as advertised. The problematic "cell" is exported as "'=SUM(1+1)". Signed-off-by: Chris Cormack <chris@bigballofwax.co.nz>
We will have to schedule this with RMaints for the next maintenance release.
Pushed for 25.05! Well done everyone, thank you!
These patches have been included in all maintained versions, see "Version(s) released in".
Now we should upgrade to the last version of DataTables and remove this from Koha I guess... It's fixed there.
(In reply to Jonathan Druart from comment #22) > Now we should upgrade to the last version of DataTables and remove this from > Koha I guess... > > It's fixed there. Shoudl we open a separate bug report for the update?