Problem/Motivation

This was previously reported to the Drupal Security Team, who cleared it to be a public issue.

#3440366: Modify formula injection warning when drupal/webform_xlsx_export is installed warned that the CSV exporter doesn't escape. But this should be possible.

Proposed resolution

Add an off-by-default escaping following the same technique used in the Symfony CSV serializer, which is to add a single quotation mark before likely formulas.

User interface changes

This updates the injection warning and adds the option.

API changes

None.

Data model changes

This adds one item to the settings schema.

Issue fork webform-3637800

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

About issue forks

Comments

cilefen created an issue. See original summary.

cilefen opened merge request !979

cilefen’s picture

cilefen commented
Issue summary: View changes
cilefen’s picture

cilefen commented
Issue summary: View changes
Related issues: +#3440366: Modify formula injection warning when drupal/webform_xlsx_export is installed
cilefen’s picture

cilefen commented
Status: Active » Needs review
liam morland’s picture

liam morland
Primary language English
Location Ontario, CA 🇨🇦
commented

I just had a quick look. Was it intentional to set escape_formulas to true in config/install/webform.settings.yml?

cilefen’s picture

cilefen commented

Yes. My position is that new installs should get the protection but we don’t opt in existing sites.

Its notable that this MR changes the escape parameter to function fputcsv from \\ to an empty string when there may be a formula, so it’s more effective. From the fputcsv’s docs:

Warning

When escape is set to anything other than an empty string ("") it can result in CSV that is not compliant with » RFC 4180 or unable to survive a roundtrip through the PHP CSV functions. The default for escape is "\\" so it is recommended to set it to the empty string explicitly. The default value will change in a future version of PHP, no earlier than PHP 9.0.

So in its current form, this MR is backwards compatible with the legacy setting when there isn’t a formula. We can discuss updating the legacy setting in all cases.