Bug 114878 - Add option to CSV import to disable formula injection
Summary: Add option to CSV import to disable formula injection
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: Calc (show other bugs)
Version:
(earliest affected)
Inherited From OOo
Hardware: All All
: medium enhancement
Assignee: Eike Rathke
URL: http://georgemauer.net/2017/10/07/csv...
Whiteboard: target:7.3.0
Keywords:
: 142536 (view as bug list)
Depends on:
Blocks: CSV-Import
  Show dependency treegraph
 
Reported: 2018-01-07 11:23 UTC by "fb_!v=
Modified: 2021-09-08 13:10 UTC (History)
6 users (show)

See Also:
Crash report or crash signature:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description "fb_!v= 2018-01-07 11:23:04 UTC
Description:
Maliciously crafted CSV document leads, in violation of RFC4180, to remote code execution.

http://georgemauer.net/2017/10/07/csv-injection.html
https://github.com/swisskyrepo/PayloadsAllTheThings/tree/master/CSV%20injection


Steps to Reproduce:
http://georgemauer.net/2017/10/07/csv-injection.html
https://github.com/swisskyrepo/PayloadsAllTheThings/tree/master/CSV%20injection


Actual Results:  
RCE

Expected Results:
No RCE. Fields must not be interpreted as formulas in such kinds of documents.


Reproducible: Always


User Profile Reset: No



Additional Info:


User-Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64; rv:59.0) Gecko/20100101 Firefox/59.0
Comment 1 Jean-Baptiste Faure 2018-01-07 17:07:17 UTC
Please, have a look at https://www.libreoffice.org/about-us/security/

@Eike: you may be interested in this bug report.

Best regards. JBF
Comment 2 Mike Kaganski 2018-01-08 01:46:14 UTC
Actually, this is not a "remote" code execution. And this doesn't differ from any other spreadsheet file being opened in a spreadsheet application, where formulas can appear. If you use XLS, or ODS, or anything, there are formulas, and they may do all the same kind of things. The only difference here is that it's not a widespread knowledge that CSV files can contain that, too, despite the RFC says otherwise.

RFC is great, but current state (with billions of existing files that require to keep working) is the de-facto standard. And that isn't gonna change. The only things required (not here, but universally) is amendment to the RFC that makes it up-to-date, and wide informing.
Comment 3 Xisco Faulí 2018-02-14 09:55:01 UTC
@Mike Kaganski, should it be closed as RESOLVED WONTFIX ?
Comment 4 Jean-Baptiste Faure 2018-02-18 22:26:53 UTC
(In reply to Xisco Faulí from comment #3)
> @Mike Kaganski, should it be closed as RESOLVED WONTFIX ?

If I understand well what Mike wrote, it should be closed as RESOLVED NOTABUG.
WONTFIX agrees there is a problem in LibreOffice. It seems it is not the case.

Best regards. JBF
Comment 5 Mike Kaganski 2018-02-19 05:38:31 UTC
Well, I'd close it as WONTFIX (after updating our help), because the issue of discrepancy between documentation and implementation indeed exists.

But I only expressed my personal opinion, and I suppose that erAck's opinion here is much more relevant.
Comment 6 Eike Rathke 2018-02-19 14:16:54 UTC
With master, 6.0.1 and 5.4.5 if a DDE() function is used in a formula imported (also from CSV) it leads to the "This file contains links to other files. Should they be updated?" dialogue and the function is only executed after confirmation. On master the modal dialogue was changed to an InfoBar (and reading "This file contains links to other files or external resources" to point out it's not only about local files) so the user can inspect Edit -> Links what external data would be accessed before confirming.
Comment 7 Eike Rathke 2018-02-19 14:38:29 UTC
We could add yet another option to the CSV import dialogue like "Import formulas as text" or some such and pre-set checked for unaware users.
Comment 8 Eike Rathke 2018-03-02 17:34:27 UTC
Adjusting title because with the current releases there is no vulnerability, executing DDE is not possible without user interaction.
Comment 9 jomo 2018-04-12 15:27:09 UTC
(In reply to Mike Kaganski from comment #2)
> this doesn't differ from any other spreadsheet file being opened in a spreadsheet application, where formulas can appear. If you use XLS, or ODS, or anything, there are formulas, and they may do all the same kind of things.

I disagree. CSV is not a "spreadsheet file" comparable to XLS or ODS. CSV is Comma-separated values (where all values are text).

When importing, say, a CSV file with a list of comments, I would not expect formulas to be executed only because a comment started with an equals sign.
Comment 10 Mike Kaganski 2021-05-28 08:28:13 UTC
*** Bug 142536 has been marked as a duplicate of this bug. ***
Comment 11 Martin Häcker 2021-05-28 08:33:13 UTC
I certainly didn't find this bug report before reporting mine. I'd like to bring over some of my comments there so they are not lost in the duplicate.

-- snip --
Steps to Reproduce:
1. Create data only css file

-- snip --
,0
0,=SUMME(1;1)
1,=WEBDIENST("http://localhost:8000")
-- snap --

2. Open with  German version of Calc (Screenshot 'import dialog with preview.png'). Observe that the preview renders all the formulas as _DATA_ as it should be.
3. Click 'import'
4. Observe Screenshot 'imported.png'

Actual Results:
The two fields are not rendered as previewed, instead they are assumed to be formulas and are executed. Luckily there seems to be a security safeguard that at least blocks the http call from immediate execution. However even this block is removed by a single click on the notice at the top of the window.

Expected Results:
I have imported a CSV file (which is a data only format), watched the file beforehand in a text editor to see what I will be getting, watched the preview for correctness and am still not getting the import that was previewed. This is highly surprising and als a huge enabler for a full class of security problems.

If I want the data to be interpreted and changed by Libre Office Calc, that needs to be a separate (off by default) check box that warns about the problems and security risks this poses - especially if the preview is not complete and therefore does not allow me to assess what checking this box would exactly do.

Several problems I see here:

a) The preview should match the actual imported data
b) It is highly surprising that importing a data only format will suddenly interpret that data and not display what is in the file. This is especially problematic if a web application exports data, that contains user controlled inputs to exchange it to other applications and it gets imported in Calc at some stage. The only workaround available is to know at export time, where the file will be imported in later, so the export can be sanitised for the importing application. This is highly unpractical and has a high likelihood of data loss / unintended data changes if the file is imported in the wrong application.
c) This is also highly surprising when one investigates the RFC for CSV: <https://datatracker.ietf.org/doc/html/rfc4180> which states:

   Security considerations:

      CSV files contain passive text data that should not pose any
      risks.  However, it is possible in theory that malicious binary
      data may be included in order to exploit potential buffer overruns
      in the program processing CSV data.  Additionally, private data
      may be shared via this format (which of course applies to any text
      data).

This has many and quite surprising security considerations - so much so, that OWASP maintains it as it's own category of security problem: <https://owasp.org/www-community/attacks/CSV_Injection>.

I learned of this because the German Corona Tracing App Luca was attacked through this vector - but also users of web applications I maintain are attackable by this problem.

I understand that this Is probably a long running convention for CSV import and has an aspect of compatibility with other spreadsheet applications. However this is a problematic behaviour for which there is no workaround when importing data into Calc, and there needs to be a strategy for fixing - but at least allowing a workaround for this.

I would like to suggest going at this in a multi step process - quite possibly stretched out over a long period. Maybe even 5-10 years - but of course I would like a faster transition period.

My suggestion is:

1. Add a setting on import that at least allows forcing Libre Office Calc to interpret all imported data literally so there is at least a workaround available immediately.
2. After some time, start warning on the import preview if the imported data contains anything that LibreOffice would like to interpret (At least formulas, but probably also data that could be auto formatted). This should explain the problem and/or link to a website that explains the problem and the security concerns.
3. After some more time, switch on this option by default and instead warn if the imported data contains interpretable data. Maybe show a preview of what the interpretation would change to allow the user to understand what this would do.

That way impact on existing users of that feature can be minimised, while still there is at least an immediate workaround available. The time bought by this measures can then be used to create the other suggested import features to make the transition to not interpreting imported CSV data by default safe for everyone.
-- snap --
Comment 12 Mike Kaganski 2021-05-28 15:21:06 UTC
(In reply to Martin Häcker from comment #11)

Only commenting one single quotation below.

> a) The preview should match the actual imported data

No. CSV dialog does not fully process the data, it only shows which data would go where. It will not match the end result, it would be undesirable, including possibility to see "empty" cells (where the result is e.g. a pair of quotes), and that would make it unclear if the data ends in proper cells or shifted - the whole purpose of the preview.
Comment 13 Mike Kaganski 2021-05-28 15:24:55 UTC
(In reply to Mike Kaganski from comment #12)
> including possibility to see "empty" cells (where the result is e.g. a pair
> of quotes)

I must admit that I was too fast to choose the example - where quotes already get hidden in the dialog. Yet, there may be similar cases (say, with the formulas); and honestly, I'd prefer that quotes also not hidden.
Comment 14 Martin Häcker 2021-05-31 06:48:56 UTC Comment hidden (off-topic)
Comment 15 Martin Häcker 2021-05-31 06:51:06 UTC Comment hidden (off-topic)
Comment 16 Commit Notification 2021-08-31 00:56:05 UTC
Eike Rathke committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/4c29c2bfab18b25229e0bc71014fd2ca42ec5bff

Resolves: tdf#114878 Add 'Evaluate formulas' option to CSV import and paste

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 17 Mike Kaganski 2021-08-31 06:50:13 UTC
(In reply to Commit Notification from comment #16)

Thanks! Needs a note in 7.3 release notes :)
Comment 18 Eike Rathke 2021-08-31 08:25:57 UTC
Of course. I'll add a few notes of the last two weeks all at once.
Comment 19 Commit Notification 2021-09-08 12:59:29 UTC
Xisco Fauli committed a patch related to this issue.
It has been pushed to "master":

https://git.libreoffice.org/core/commit/11622c2eb319bb096d4d587ea06a932ba5fceafb

tdf#114878: sc: Add UItest

It will be available in 7.3.0.

The patch should be included in the daily builds available at
https://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
https://wiki.documentfoundation.org/Testing_Daily_Builds

Affected users are encouraged to test the fix and report feedback.
Comment 20 Martin Häcker 2021-09-08 13:10:51 UTC
🍁🎉💥🌈