Strings starting with "=" creates unwanted formulas

Created on 7 March 2022, almost 3 years ago
Updated 18 September 2024, 3 months ago

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

-

πŸ› Bug report
Status

Fixed

Version

1.0

Component

Code

Created by

πŸ‡³πŸ‡±Netherlands mike.vindicate

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • πŸ‡³πŸ‡ΏNew Zealand RoSk0 Wellington

    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.

  • Status changed to Needs review over 1 year ago
  • Open in Jenkins β†’ Open on Drupal.org β†’
    Core: 10.1.x + Environment: PHP 8.1 & MySQL 5.7
    last update over 1 year ago
    4 pass
  • πŸ‡³πŸ‡ΏNew Zealand RoSk0 Wellington

    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.

  • πŸ‡¦πŸ‡ΊAustralia larowlan πŸ‡¦πŸ‡ΊπŸ.au GMT+10

    The security team agreed to keep this in public

  • Status changed to Needs work over 1 year ago
  • πŸ‡©πŸ‡ͺGermany FeyP

    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.

  • 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 @.

  • I think it would be best to follow the guidelines from OWASP, and to prefix such data with '

  • First commit to issue fork.
  • πŸ‡³πŸ‡ΏNew Zealand ericgsmith

    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.

  • Pipeline finished with Failed
    4 months ago
    Total: 208s
    #271049
  • Status changed to Fixed 4 months ago
  • πŸ‡³πŸ‡±Netherlands theemstra
  • Regarding #11, are there any plans to format values starting with the other recommended characters?

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

Production build 0.71.5 2024