Problem/Motivation

When you export a value which starts with an "=" it creates an formula or breaks the export because of an invalid formula.

Steps to reproduce

Create a value to export that starts with an "=". If you do this for an e-mail field which requires an "@" it will break the export.

Proposed resolution

By default escape values that start with an "=". If needed in the interface create an option that will allow formulas for certain field values.

Remaining tasks

Escape unwanted formulas
Handle the interface option

User interface changes

API changes

-

Data model changes

-

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:

Comments

mike.vindicate created an issue. See original summary.

michael.acampora’s picture

StatusFileSize
new403 bytes

Attached patch will escape unwanted formulas.

lendude’s picture

Status: Active » Needs review
StatusFileSize
new400 bytes

#2 didn't go far enough for the error we saw, I needed to quote the whole thing to get it past the value triggering the errors

anneke_vde’s picture

Status: Needs review » Reviewed & tested by the community

I tested the last patch, the batch export now runs ok and the exported data looks good.

rosk0’s picture

Version: 8.x-1.2 » 8.x-1.x-dev

I'm curious if the double quote character is the correct one to be used here.

We were recommended to use a single quote character to escape field data starting with equals sign by the security company to avoid potential remote code execution.

rosk0’s picture

Status: Reviewed & tested by the community » Needs review
StatusFileSize
new1.2 KB

When setting a cell value, PhpOffice\PhpSpreadsheet will use default value binder (\PhpOffice\PhpSpreadsheet\Cell\DefaultValueBinder), if none provided. This binder is responsible for guessing data type based on its value and this is where string starting from equals sign is getting a formula data type assigned.

I don't like current implementation because it modifies original data. I believe there is a better way, or even two.

During testing, I noticed that most office suites would prepend the data with a single quote to treat the value as a plain string and prevent formula processing. At the same time this data would be rendered in a cell without the prepended single quote. We could use this option if we really want to modify original data.

A better option, I believe, would be to tell PhpOffice\PhpSpreadsheet , and consuming office software, to treat values starting from equals sign as string and prevent formula processing. The user would than need to explicitly enable formula processing for the affected cell. This is far better position from the security point of view, in my opinion.

Tested in:

  • LibreOffice
  • WPS Office
  • SoftMaker Office
  • MS Office

Latter option is implemented in this patch.

larowlan’s picture

Going to unpublish this as its a security issue - https://owasp.org/www-community/attacks/CSV_Injection

Can we get a private issue for this on security.drupal.org?

larowlan’s picture

larowlan’s picture

The security team agreed to keep this in public

feyp’s picture

Status: Needs review » Needs work

Note that escaping the equal sign is not enough to prevent CSV injection. OWASP has a list of characters to escape, @larowlan already posted that one. This list is also used by Smyfony in its serialization component: https://symfony.com/blog/cve-2021-41270-prevent-csv-injection-via-formulas So the patch most likely needs to be updated to include the additional characters. Setting to "Needs work" for that.

solideogloria’s picture

Quoting relevant portion here:

Symfony now follows the OWASP recommendations and use the single quote ' to prefix formulas and adds the prefix to cells starting by \t, \r as well as =, +, - and @.

solideogloria’s picture

I think it would be best to follow the guidelines from OWASP, and to prefix such data with ', rather than doing the approach in #6, though I don't really know much on the subject matter or about this module.

ericgsmith made their first commit to this issue’s fork.

ericgsmith’s picture

Moved patch from #6 to MR and rebased against 8.x-1.x.

Setting MR as draft and leaving as needs work as have not looked at suggested changes in above comments 10 to 12 yet.

  • theemstra committed 43b50814 on 8.x-1.x
    Issue #3268139 by rosk0: Strings starting with "=" creates unwanted...

theemstra’s picture

Status: Needs work » Fixed
solideogloria’s picture

Regarding #11, are there any plans to format values starting with the other recommended characters?

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.