Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Sanitise or escape data when exporting to CSV #11450

Open
GuySartorelli opened this issue Oct 31, 2024 · 0 comments
Open

Sanitise or escape data when exporting to CSV #11450

GuySartorelli opened this issue Oct 31, 2024 · 0 comments

Comments

@GuySartorelli
Copy link
Member

GuySartorelli commented Oct 31, 2024

Note

Note that this is explicitly not a security vulnerability in Silverstripe CMS and therefore is not treated as one for our security processes. Instead, this is discussion of a possible security enhancement to prevent data exported from Silverstripe CMS projects from being used to exploit vulnerabilities in other unrelated software.

Currently there is no sanitisation or escaping of data that gets exported as CSV files.
There are, apparently, some vulnerabilities in spreadsheet software that can result in unsanitised/unescaped data causing problems in the user's system.

We should consider whether it's appropriate to escape or sanitise data that gets exported to CSV files using CsvBulkLoader and any other mechanisms that are built into Silverstripe CMS.

Important

PHP does have built-in fgetcsv and fputcsv functions for reading and writing CSV files, and those functions have a parameter for an "escape charaacter".... but both functions also explicitly say "it is recommended to set it to the empty string explicitly" which disables escaping.

The concern is that escaping values and then importing that same file could result in importing values different to what was originally exported. We'll need to take care to avoid that problem if we do choose to move forward with this.

Note that if this is implemented in a minor release it'll need to be configurable, and be off by default. Otherwise people may have problems importing previously exported data for example.

relevant links

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

1 participant