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

Escape formulas in XLSX #38

Merged
merged 8 commits into from
Jan 30, 2022
Merged

Escape formulas in XLSX #38

merged 8 commits into from
Jan 30, 2022

Conversation

livedo
Copy link
Contributor

@livedo livedo commented Jan 15, 2022

This adds an escape_formulas option to the XLSX generating methods in order to avoid formula injection when generating XLSX. The header row is additionally always escaped.

Usage:

SpreadsheetArchitect.to_xlsx(data: [["=1+1"]], escape_formulas: true) #default
SpreadsheetArchitect.to_xlsx(data: [["Safe"]], escape_formulas: false)
SpreadsheetArchitect.to_xlsx(data: [["=1+1", "Safe"]], escape_formulas: [true, false])

@livedo livedo marked this pull request as ready for review January 15, 2022 09:42
@livedo livedo changed the title Escape formulas Escape formulas in XLSX Jan 15, 2022
@westonganger
Copy link
Owner

My initial though was that I dont want to burden this library with every single option that axlsx has built in. I almost think you could just manually escape any user inputted string that you think may nefarious.

All axlsx does is check if the cells value starts with an "=" to determine if its a formula. So all you would have to do to escape any user input text is add a single quote to the beginning of the text. Ex. sanitized_val = val.start_with?("=") ? "'#{val}" : val

That being said I am sure many apps simply dont use formulas at all in their spreadsheet so this would actually be a good feature.

I have made a few comments and we can go from there.

@livedo
Copy link
Contributor Author

livedo commented Jan 17, 2022

@westonganger could you approve the workflows/build to run in this PR?

@livedo
Copy link
Contributor Author

livedo commented Jan 17, 2022

That being said I am sure many apps simply dont use formulas at all in their spreadsheet so this would actually be a good feature.

Yeah, I'd figure that creating spreadsheets where some of the fields contain direct user input (names and addresses and such) are a big majority compared to use cases that involve formulas, so it would make all the more sense to provide security by default.

@livedo livedo requested a review from westonganger January 19, 2022 06:42
@@ -104,7 +104,7 @@ def to_axlsx_package(opts={}, package=nil)
end
end

sheet.add_row row_data, style: styles, types: types
sheet.add_row row_data, style: styles, types: types, escape_formulas: options[:escape_formulas]
Copy link
Owner

Choose a reason for hiding this comment

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

Can we make the headers rows use escape_formulas: options[:escape_formulas] just the same as the data rows

@westonganger westonganger merged commit 50b3f81 into westonganger:master Jan 30, 2022
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.

2 participants