Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(csv-stringify): Add escape_formulas to defend against injection attacks #380

Merged
merged 2 commits into from Mar 3, 2023

Conversation

atlanteh
Copy link
Contributor

@atlanteh atlanteh commented Feb 28, 2023

This PR allows setting simple parameter to defend against CSV injection attacks by adding escape_formulas parameter that escapes values that start with =, +, -, @, \t, or \r with a '

@atlanteh
Copy link
Contributor Author

atlanteh commented Mar 1, 2023

@wdavidw Can you please review?

packages/csv-stringify/lib/api/index.js Show resolved Hide resolved
@@ -158,6 +158,9 @@ const stringifier = function(options, state, info){
}
});
quotedMatch = quotedMatch && quotedMatch.length > 0;
if (escape_formulas && ['=', '+', '-', '@', '\t', '\r'].includes(value[0])) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Shouldn't this happen in the __onField function ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think __onField is only in csv-parse. not csv-stringify

packages/csv-stringify/test/option.escape_formulas.coffee Outdated Show resolved Hide resolved
@atlanteh
Copy link
Contributor Author

atlanteh commented Mar 2, 2023

@wdavidw PR updated according to your review

@wdavidw
Copy link
Member

wdavidw commented Mar 3, 2023

Thank you for your contribution, I'll take it from there.

@wdavidw wdavidw merged commit 47ac4bd into adaltas:master Mar 3, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants