-
Notifications
You must be signed in to change notification settings - Fork 3.2k
Add optional formula sanitization for csv export to prevent CSV Injection attacks #11592
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
Conversation
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit 27fc366:
|
…eature/dev-issue-1275
|
Launch the local version of documentation by running: npm run docs:review f7bf68f2ca3930f5bd3542f776eb1b5b65352e9c |
| * @returns {string} | ||
| * @private | ||
| */ | ||
| _sanitizeValueWithOWASP(value) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not crucial, but you could utilize the private method notation with #, so #sanitizeValueWithOWASP. I think this plugin doesn't use it yet because it was created before we supported it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sequba I can see this change broke the test. These "private" functions don't seem to be that private 🙄 . I think you can either revert this change or try to refactor the plugin to make them actually private, whatever you think it's best.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I changed escapeCell to be public again
docs/content/guides/accessories-and-menus/export-to-csv/export-to-csv.md
Outdated
Show resolved
Hide resolved
…eature/dev-issue-1275
This will be done in separate task: https://github.com/handsontable/dev-handsontable/issues/2444 |

Changes:
sanitizeValuestoexportPlugin.downloadFileAPI that controls the value sanitization during the CSV exportsanitizeValuesistrue, HOT sanitizes values according to OWASP recommendationssanitizeValuesis a regexp, HOT escapes all values that match the regexpsanitizeValuesis a function, HOT replaces all values with the return value of the functionsanitizeValuesis not set, HOT doesn't sanitize valuesExportOptionsinterfaceHow has this been tested?
Types of changes
Related issue(s):
Fixes https://github.com/handsontable/dev-handsontable/issues/1275
Affected project(s):
handsontable@handsontable/angular@handsontable/react@handsontable/react-wrapper@handsontable/vue@handsontable/vue3Checklist: