Protection against CSV injection attacks

[wcerfgba]

When importing a CSV file, Microsoft Excel and LibreOffice Calc will both interpret cells beginning with a = as formulae, which can lead to attacks that can result in data exfiltration or arbitrary command execution. [1] This is easily remedied by prefixing cells that begin with =, +, - or @ with a ' in order to suppress automatic interpretation of formulae by these softwares. [2]

I would like to propose an option escapeFormulae for Papa.unparse to provide this prefixing behaviour.

Thanks!

[1] https://owasp.org/www-community/attacks/CSV_Injection
[2] https://www.contextis.com/en/blog/comma-separated-vulnerabilities

[pokoli]

That make sense for me.

I'm wondering if we should activate this option by default or not.

[wcerfgba]

I'm wondering if we should activate this option by default or not.

I guess this is dependent on if you have users who currently rely on formulae interpretation and for whom this would break on an upgrade. My intuition is that this would be a minority of users and that for most use cases, it would be better to be safe by default, and have escapeFormulae default to true.

[pokoli]

Having a safer default sounds good to me. Then we just need to have a proper documentation to explain why the should should be desactivated and include a warning about the it's safety risks.

So we just need an implmentation for it.

[dylanlingelbach]

Having a safer default sounds good to me

I'm sorry but this doesn't make sense to me. papaparse is a CSV parser. By default modifying the contents of a CSV file that may or may not ever get processed by Excel/LibreOffice seems like very odd behavior.

This bug is reported as a high severity vulnerability which also doesn't make sense to me - by that logic every text editor has the same vulnerability

[pokoli]

@dylanlingelbach not all contents are modified but only the ones that are suceptible of having a formula, just the ones that start with = which from my understanding should not me the bast majority.

But I agree that this should not be treated as high severty (at least on Papaparse) because:

• The file may not be openend by a SpreadSheet software, so this users are not afected.
• There is no evidence how the attacker can gain access to the input data to include the formulas and how to let the user execute it.

P.S: My main usage of papaparse is to generate files to be opened by a SpreadSheet software, so probably I'm a little bit biaged here. I will like to have more opinion on this.

@mholt @dboskovic what do you think it should be the defalut value of this new parameter?

[asafbiton]

Hi there! 👋

My name is Asaf and I'm part of the Snyk Security Team. We have been tracking this issue for a few days now, and an advisory has been mistakenly published. I tend to agree with all the above arguments and do not believe there is a vuln within the context of papaparse.

I have therefore revoked this advisory from our database. I apologize for any inconvenience caused by this.

For further inquiries please don't hesitate to contact us at report@snyk.io or using the vulnerability disclosure form.

[pokoli]

@asafbiton Thank you so much!

[dylanlingelbach]

@asafbiton thank you!

@pokoli I think if any contents of the CSV file are modified by default that is odd (CSV quoting/escaping excluded)

[wcerfgba]

Sorry I have been away from the thread for a few days. Would it be fair to say the consensus is that this option is a good idea, but it should be disabled by default? I am happy to write a patch some point this week if we are aligned on this. :)