Repository: mholt/PapaParse
Pull request #904 (2021-12-05 12:38:04)

Add more cases to escapeFormulae, allow to pass RegExp
See #796 #793

@pokoli @wcerfgba Hey guys, could you check this PR?

It adds more cases for `escapeFormulae` option, cf https://owasp.org/www-community/attacks/CSV_Injection

caub: I added possibility to pass a RegExp as `escapeFormulae` option, but not sure it's needed, just tell me, I can remove that
pokoli: Hi @caub!

Thanks for your suggestion. Could you please keep the current tests cases as is (so we ensure nothing is borken) and add a new test case for the regexp validation (the new feature) ?

Thanks in advance!
caub: Hey @pokoli 
Current changes are actually changed, https://owasp.org/www-community/attacks/CSV_Injection recommanded to escape values anyway as soon as a formulae is used

Not sure if really necessary but what's your thought? can we revert my last commit if you prefer? (and then previous tests will be identical)
pokoli: @caub feel free to remove some test suit if it is no longer valid (but please keep the valid ones)
caub: ok done, https://github.com/mholt/PapaParse/pull/904/files you can see 2 of the previous tests are affected, because  for example `=test` was converted in `'=test` as it doesn't require escaping the whole value, with owasp recommandation we get either `"'=test"` or `''=test'`
pokoli: Hi @caub, 

Tests failed due to node15 (which seems unrelated). I removed the version from github actions in https://github.com/mholt/PapaParse/commit/26a86fdf9f2e97225b101dad18dacdbac517ce58 

Could you please rebase the PR so I can relauch the test suite to ensure everything is in place. 

Sorry for the inconvenience. 
caub: yep, done

Patch:
diff --git a/docs/docs.html b/docs/docs.html
index c2d82bd..cf198a5 100644
--- a/docs/docs.html
+++ b/docs/docs.html
@@ -343,7 +343,7 @@
 									<code>escapeFormulae</code>
 								</td>
 								<td>
-									If <code>true</code>, field values that begin with <code>=</code>, <code>+</code>, <code>-</code>, or <code>@</code>, will be prepended with a <code>'</code> to defend against <a href="https://www.contextis.com/en/blog/comma-separated-vulnerabilities" target="_blank" rel="noopener">injection attacks</a>, because Excel and LibreOffice will automatically parse such cells as formulae.
+									If <code>true</code>, field values that begin with <code>=</code>, <code>+</code>, <code>-</code>, <code>@</code>, <code>\t</code>, or <code>\r</code>, will be prepended with a <code>'</code> to defend against <a href="https://owasp.org/www-community/attacks/CSV_Injection" target="_blank" rel="noopener">injection attacks</a>, because Excel and LibreOffice will automatically parse such cells as formulae. You can override those values by setting this option to a regular expression
 								</td>
 							</tr>
 						</table>
diff --git a/papaparse.js b/papaparse.js
index 057a16c..91de82f 100755
--- a/papaparse.js
+++ b/papaparse.js
@@ -367,11 +367,11 @@ License: MIT
 				_escapedQuote = _config.escapeChar + _quoteChar;
 			}
 
-			if (typeof _config.escapeFormulae === 'boolean')
-				_escapeFormulae = _config.escapeFormulae;
+			if (typeof _config.escapeFormulae === 'boolean' || _config.escapeFormulae instanceof RegExp) {
+				_escapeFormulae = _config.escapeFormulae instanceof RegExp ? _config.escapeFormulae : /^[=+\-@\t\r].*$/;
+			}
 		}
 
-
 		/** The double for loop that iterates the data and writes out a CSV string including header row */
 		function serialize(fields, data, skipEmptyLines)
 		{
@@ -444,13 +444,17 @@ License: MIT
 			if (str.constructor === Date)
 				return JSON.stringify(str).slice(1, 25);
 
-			if (_escapeFormulae === true && typeof str === "string" && (str.match(/^[=+\-@].*$/) !== null)) {
+			var needsQuotes = false;
+
+			if (_escapeFormulae && typeof str === "string" && _escapeFormulae.test(str)) {
 				str = "'" + str;
+				needsQuotes = true;
 			}
 
 			var escapedQuoteStr = str.toString().replace(quoteCharRegex, _escapedQuote);
 
-			var needsQuotes = (typeof _quotes === 'boolean' && _quotes)
+			needsQuotes = needsQuotes
+							|| _quotes === true
 							|| (typeof _quotes === 'function' && _quotes(str, col))
 							|| (Array.isArray(_quotes) && _quotes[col])
 							|| hasAny(escapedQuoteStr, Papa.BAD_DELIMITERS)


Test patch:
diff --git a/tests/test-cases.js b/tests/test-cases.js
index 0c3a62d..ab806ba 100644
--- a/tests/test-cases.js
+++ b/tests/test-cases.js
@@ -1881,7 +1881,7 @@ var UNPARSE_TESTS = [
 		description: "Escape formulae",
 		input: [{ "Col1": "=danger", "Col2": "@danger", "Col3": "safe" }, { "Col1": "safe=safe", "Col2": "+danger", "Col3": "-danger, danger" }, { "Col1": "'+safe", "Col2": "'@safe", "Col3": "safe, safe" }],
 		config: { escapeFormulae: true },
-		expected: 'Col1,Col2,Col3\r\n\'=danger,\'@danger,safe\r\nsafe=safe,\'+danger,"\'-danger, danger"\r\n\'+safe,\'@safe,"safe, safe"'
+		expected: 'Col1,Col2,Col3\r\n"\'=danger","\'@danger",safe\r\nsafe=safe,"\'+danger","\'-danger, danger"\r\n\'+safe,\'@safe,"safe, safe"'
 	},
 	{
 		description: "Don't escape formulae by default",
@@ -1898,7 +1898,7 @@ var UNPARSE_TESTS = [
 		description: "Escape formulae with single-quote quoteChar and escapeChar",
 		input: [{ "Col1": "=danger", "Col2": "@danger", "Col3": "safe" }, { "Col1": "safe=safe", "Col2": "+danger", "Col3": "-danger, danger" }, { "Col1": "'+safe", "Col2": "'@safe", "Col3": "safe, safe" }],
 		config: { escapeFormulae: true, quoteChar: "'", escapeChar: "'" },
-		expected: 'Col1,Col2,Col3\r\n\'\'=danger,\'\'@danger,safe\r\nsafe=safe,\'\'+danger,\'\'\'-danger, danger\'\r\n\'\'+safe,\'\'@safe,\'safe, safe\''
+		expected: 'Col1,Col2,Col3\r\n\'\'\'=danger\',\'\'\'@danger\',safe\r\nsafe=safe,\'\'\'+danger\',\'\'\'-danger, danger\'\r\n\'\'+safe,\'\'@safe,\'safe, safe\''
 	},
 	{
 		description: "Escape formulae with single-quote quoteChar and escapeChar and forced quotes",
@@ -1906,6 +1906,31 @@ var UNPARSE_TESTS = [
 		config: { escapeFormulae: true, quotes: true, quoteChar: "'", escapeChar: "'" },
 		expected: '\'Col1\',\'Col2\',\'Col3\'\r\n\'\'\'=danger\',\'\'\'@danger\',\'safe\'\r\n\'safe=safe\',\'\'\'+danger\',\'\'\'-danger, danger\'\r\n\'\'\'+safe\',\'\'\'@safe\',\'safe, safe\''
 	},
+	// new escapeFormulae values:
+	{
+		description: "Escape formulae with tab and carriage-return",
+		input: [{ "Col1": "\tdanger", "Col2": "\rdanger,", "Col3": "safe\t\r" }],
+		config: { escapeFormulae: true },
+		expected: 'Col1,Col2,Col3\r\n"\'\tdanger","\'\rdanger,","safe\t\r"'
+	},
+	{
+		description: "Escape formulae with tab and carriage-return, with forced quotes",
+		input: [{ "Col1": "	danger", "Col2": "\rdanger,", "Col3": "safe\t\r" }],
+		config: { escapeFormulae: true, quotes: true },
+		expected: '"Col1","Col2","Col3"\r\n"\'\tdanger","\'\rdanger,","safe\t\r"'
+	},
+	{
+		description: "Escape formulae with tab and carriage-return, with single-quote quoteChar and escapeChar",
+		input: [{ "Col1": "	danger", "Col2": "\rdanger,", "Col3": "safe, \t\r" }],
+		config: { escapeFormulae: true, quoteChar: "'", escapeChar: "'" },
+		expected: 'Col1,Col2,Col3\r\n\'\'\'\tdanger\',\'\'\'\rdanger,\',\'safe, \t\r\''
+	},
+	{
+		description: "Escape formulae with tab and carriage-return, with single-quote quoteChar and escapeChar and forced quotes",
+		input: [{ "Col1": "	danger", "Col2": "\rdanger,", "Col3": "safe, \t\r" }],
+		config: { escapeFormulae: true, quotes: true, quoteChar: "'", escapeChar: "'" },
+		expected: '\'Col1\',\'Col2\',\'Col3\'\r\n\'\'\'\tdanger\',\'\'\'\rdanger,\',\'safe, \t\r\''
+	},
 ];
 
 describe('Unparse Tests', function() {