From 0746815b75296bcf65d49a66f0dca1427ac65f3e Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Fri, 31 Mar 2023 04:40:41 +0900 Subject: Add settings for escape_formulas at global, workbook, worksheet, row and cell levels. --- README.md | 20 +++++++++++ examples/escape_formula_example.md | 20 ++++++++++- lib/axlsx.rb | 15 ++++++++ lib/axlsx/workbook/workbook.rb | 47 ++++++++++++++----------- lib/axlsx/workbook/worksheet/cell.rb | 26 +++++++------- lib/axlsx/workbook/worksheet/row.rb | 18 +++++++--- lib/axlsx/workbook/worksheet/worksheet.rb | 21 +++++++++-- test/workbook/tc_workbook.rb | 36 +++++++++++++++++++ test/workbook/worksheet/tc_row.rb | 26 ++++++++++++++ test/workbook/worksheet/tc_worksheet.rb | 58 ++++++++++++++++++++++++------- 10 files changed, 233 insertions(+), 54 deletions(-) diff --git a/README.md b/README.md index 697b71d7..2c1076c5 100644 --- a/README.md +++ b/README.md @@ -119,6 +119,26 @@ Currently the following additional gems are available: - [activeadmin-caxlsx](https://github.com/caxlsx/activeadmin-caxlsx) * An Active Admin plugin that includes DSL to create downloadable reports. +## Security + +To prevent [Formula Injection](https://www.owasp.org/index.php/CSV_Injection) vulnerabilities, set the following in an initializer: + +```ruby +Axlsx.escape_formulas = true +``` + +Then, set the following on each cell you'd like to add a formula: + +```ruby +cell.escape_formulas = true +``` + +Refer to examples/escape_formula.md for how to set `escape_formulas` on the workbook, worksheet, row and/or cell level. + +**Important:** The global setting `Axlsx.escape_formulas = true` will become the default in the next major release (Axlsx 4.0). +If you do not wish to set `Axlsx.escape_formulas = true` now, at a minimum, please set `Axlsx.escape_formulas = false` to +ensure continuity when upgrading. + ## Known Software Interoperability Issues As axslx implements the Office Open XML (ECMA-376 spec) much of the diff --git a/examples/escape_formula_example.md b/examples/escape_formula_example.md index fb23ce66..9a8efc21 100644 --- a/examples/escape_formula_example.md +++ b/examples/escape_formula_example.md @@ -1,14 +1,30 @@ ## Description -You could escape formulas +You may escape formulas using `escape_formulas` on the global, workbook, worksheet, row and/or cell level. +This is used to prevent [Formula Injection](https://www.owasp.org/index.php/CSV_Injection) vulnerabilities. + +The following are possible: + +| Scope | Example | Notes | +|-----------|--------------------------------------------------------------------------|--------------------------------------------------------------------------------------------| +| Global | `Axlsx.escape_formulas = true` | Affects worksheets created *after* setting. Does not affect existing worksheets. | +| Workbook | `workbook.escape_formulas = true` | Affects child worksheets added *after* setting. Does not affect existing child worksheets. | +| Worksheet | `workbook.add_worksheet(name: 'Name', escape_formulas: true)` | | +| Worksheet | `worksheet.worksheet = true` | Affects child rows/cells added *after* setting. Does not affect existing child rows/cells. | +| Row | `worksheet.add_row(['=FOO()', '=BAR()], escape_formulas: [true, false])` | Can specify as either Boolean (all cells) or Array (one value per cell). | +| Row | `row.escape_formulas = [true, false]` | Changes the `escape_formulas` value on existing cells. Can use either Boolean or Array. | +| Cell | `cell.escape_formulas = true` | | ## Code ```ruby require 'axlsx' +Axlsx.escape_formulas = true + p = Axlsx::Package.new wb = p.workbook +wb.escape_formulas #=> true (initial value will be Axlsx.escape_formulas) wb.add_worksheet(name: 'Escaping Formulas') do |sheet| sheet.add_row [1, 2, 3, '=SUM(A2:C2)'], escape_formulas: true @@ -17,6 +33,8 @@ wb.add_worksheet(name: 'Escaping Formulas') do |sheet| '=IF(13+13=4,4,5)', '=IF(99+99=4,4,5)' ], escape_formulas: [true, false, true] + + sheet.rows.first.cells.first.escape_formulas = false end p.serialize 'escape_formula_example.xlsx' diff --git a/lib/axlsx.rb b/lib/axlsx.rb index 23082b71..c3532b72 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -201,4 +201,19 @@ module Axlsx def self.trust_input=(trust_me) @trust_input = trust_me end + + # Whether to treat values starting with an equals sign as formulas or as literal strings. + # Allowing user-generated data to be interpreted as formulas is a security risk. + # See https://www.owasp.org/index.php/CSV_Injection for details. + # @return [Boolean] + def self.escape_formulas + @escape_formulas || false + end + + # Sets whether to treat values starting with an equals sign as formulas or as literal strings. + # @param [Boolean] value The value to set. + def self.escape_formulas=(value) + Axlsx.validate_boolean(value) + @escape_formulas = value + end end diff --git a/lib/axlsx/workbook/workbook.rb b/lib/axlsx/workbook/workbook.rb index 938d4aee..3be95c8f 100644 --- a/lib/axlsx/workbook/workbook.rb +++ b/lib/axlsx/workbook/workbook.rb @@ -1,5 +1,3 @@ -# -*- coding: utf-8 -*- -module Axlsx require 'axlsx/workbook/worksheet/sheet_calc_pr.rb' require 'axlsx/workbook/worksheet/auto_filter/auto_filter.rb' require 'axlsx/workbook/worksheet/date_time_converter.rb' @@ -42,8 +40,6 @@ require 'axlsx/workbook/worksheet/row_breaks' require 'axlsx/workbook/worksheet/col_breaks' require 'axlsx/workbook/workbook_view' require 'axlsx/workbook/workbook_views' - - require 'axlsx/workbook/worksheet/worksheet.rb' require 'axlsx/workbook/shared_strings_table.rb' require 'axlsx/workbook/defined_name.rb' @@ -60,6 +56,9 @@ require 'axlsx/workbook/worksheet/sheet_view.rb' require 'axlsx/workbook/worksheet/sheet_format_pr.rb' require 'axlsx/workbook/worksheet/pane.rb' require 'axlsx/workbook/worksheet/selection.rb' + +module Axlsx + # The Workbook class is an xlsx workbook that manages worksheets, charts, drawings and styles. # The following parts of the Office Open XML spreadsheet specification are not implimented in this version. # @@ -109,29 +108,28 @@ require 'axlsx/workbook/worksheet/selection.rb' @is_reversed = v end - - # A collection of worksheets associated with this workbook. + # A collection of worksheets associated with this workbook. # @note The recommended way to manage worksheets is add_worksheet # @see Workbook#add_worksheet # @see Worksheet # @return [SimpleTypedList] attr_reader :worksheets - # A colllection of charts associated with this workbook + # A collection of charts associated with this workbook # @note The recommended way to manage charts is Worksheet#add_chart # @see Worksheet#add_chart # @see Chart # @return [SimpleTypedList] attr_reader :charts - # A colllection of images associated with this workbook + # A collection of images associated with this workbook # @note The recommended way to manage images is Worksheet#add_image # @see Worksheet#add_image # @see Pic # @return [SimpleTypedList] attr_reader :images - # A colllection of drawings associated with this workbook + # A collection of drawings associated with this workbook # @note The recommended way to manage drawings is Worksheet#add_chart # @see Worksheet#add_chart # @see Drawing @@ -140,15 +138,14 @@ require 'axlsx/workbook/worksheet/selection.rb' # pretty sure this two are always empty and can be removed. - - # A colllection of tables associated with this workbook + # A collection of tables associated with this workbook # @note The recommended way to manage drawings is Worksheet#add_table # @see Worksheet#add_table # @see Table # @return [SimpleTypedList] attr_reader :tables - # A colllection of pivot tables associated with this workbook + # A collection of pivot tables associated with this workbook # @note The recommended way to manage drawings is Worksheet#add_table # @see Worksheet#add_table # @see Table @@ -217,11 +214,9 @@ require 'axlsx/workbook/worksheet/selection.rb' self.styles_applied = true end - # Indicates if the epoc date for serialization should be 1904. If false, 1900 is used. @@date1904 = false - # A quick helper to retrive a worksheet by name # @param [String] name The name of the sheet you are looking for # @return [Worksheet] The sheet found, or nil @@ -231,7 +226,8 @@ require 'axlsx/workbook/worksheet/selection.rb' end # Creates a new Workbook - # The recomended way to work with workbooks is via Package#workbook + # The recommended way to work with workbooks is via Package#workbook + # @option options [Boolean] date1904. If this is not specified, date1904 is set to false. Office 2011 for Mac defaults to false. # @option options [Boolean] date1904. If this is not specified, date1904 is set to false. Office 2011 for Mac defaults to false. def initialize(options={}) @styles = Styles.new @@ -243,13 +239,12 @@ require 'axlsx/workbook/worksheet/selection.rb' @tables = SimpleTypedList.new Table @pivot_tables = SimpleTypedList.new PivotTable @comments = SimpleTypedList.new Comments - - @use_autowidth = true @bold_font_multiplier = BOLD_FONT_MULTIPLIER @font_scale_divisor = FONT_SCALE_DIVISOR - self.date1904= !options[:date1904].nil? && options[:date1904] + self.escape_formulas = options[:escape_formulas].nil? ? Axlsx.escape_formulas : options[:escape_formulas] + self.date1904 = !options[:date1904].nil? && options[:date1904] yield self if block_given? end @@ -268,6 +263,19 @@ require 'axlsx/workbook/worksheet/selection.rb' # @return [Boolean] def self.date1904() @@date1904; end + # Whether to treat values starting with an equals sign as formulas or as literal strings. + # Allowing user-generated data to be interpreted as formulas is a security risk. + # See https://www.owasp.org/index.php/CSV_Injection for details. + # @return [Boolean] + attr_reader :escape_formulas + + # Sets whether to treat values starting with an equals sign as formulas or as literal strings. + # @param [Boolean] value The value to set. + def escape_formulas=(value) + Axlsx.validate_boolean(value) + @escape_formulas = value + end + # Indicates if the workbook should use autowidths or not. # @note This gem no longer depends on RMagick for autowidth # calculation. Thus the performance benefits of turning this off are @@ -380,7 +388,7 @@ require 'axlsx/workbook/worksheet/selection.rb' # @param [Symbol] space must be one of :preserve or :default def xml_space=(space) Axlsx::RestrictionValidator.validate(:xml_space, [:preserve, :default], space) - @xml_space = space; + @xml_space = space end # returns a range of cells in a worksheet @@ -420,6 +428,5 @@ require 'axlsx/workbook/worksheet/selection.rb' end str << '' end - end end diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 125ca051..8e55d0c5 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -30,10 +30,9 @@ module Axlsx # @option options [String] color an 8 letter rgb specification # @option options [Number] formula_value The value to cache for a formula cell. # @option options [Symbol] scheme must be one of :none, major, :minor - # @option options [Boolean] escape_formulas - Whether to treat a value starting with an equal - # sign as formula (default) or as simple string. - # Allowing user generated data to be interpreted as formulas can be dangerous - # (see https://www.owasp.org/index.php/CSV_Injection for details). + # @option options [Boolean] escape_formulas Whether to treat values starting with an equals + # sign as formulas or as literal strings. Allowing user-generated data to be interpreted as + # formulas is a security risk. See https://www.owasp.org/index.php/CSV_Injection for details. def initialize(row, value = nil, options = {}) @row = row # Do not use instance vars if not needed to use less RAM @@ -42,15 +41,13 @@ module Axlsx type = options.delete(:type) || cell_type_from_value(value) self.type = type unless type == :string - escape_formulas = options[:escape_formulas] - self.escape_formulas = escape_formulas unless escape_formulas.nil? - val = options.delete(:style) self.style = val unless val.nil? || val == 0 val = options.delete(:formula_value) self.formula_value = val unless val.nil? parse_options(options) + self.escape_formulas = row.worksheet.escape_formulas if escape_formulas.nil? self.value = value value.cell = self if contains_rich_text? @@ -134,16 +131,17 @@ module Axlsx self.value = @value unless !defined?(@value) || @value.nil? end - # Whether to treat a value starting with an equal - # sign as formula (default) or as simple string. - # Allowing user generated data to be interpreted as formulas can be dangerous - # (see https://www.owasp.org/index.php/CSV_Injection for details). + # Whether to treat values starting with an equals sign as formulas or as literal strings. + # Allowing user-generated data to be interpreted as formulas is a security risk. + # See https://www.owasp.org/index.php/CSV_Injection for details. # @return [Boolean] attr_reader :escape_formulas - def escape_formulas=(v) - Axlsx.validate_boolean(v) - @escape_formulas = v + # Sets whether to treat values starting with an equals sign as formulas or as literal strings. + # @param [Boolean] value The value to set. + def escape_formulas=(value) + Axlsx.validate_boolean(value) + @escape_formulas = value end # The value of this cell. diff --git a/lib/axlsx/workbook/worksheet/row.rb b/lib/axlsx/workbook/worksheet/row.rb index 63a8d328..abef95cb 100644 --- a/lib/axlsx/workbook/worksheet/row.rb +++ b/lib/axlsx/workbook/worksheet/row.rb @@ -24,6 +24,7 @@ module Axlsx # @option options [Array] values # @option options [Array, Symbol] types # @option options [Array, Integer] style + # @option options [Array, Boolean] escape_formulas # @option options [Float] height the row's height (in points) # @option options [Integer] offset - add empty columns before values # @see Row#array_to_cells @@ -104,20 +105,29 @@ module Axlsx c end - # sets the color for every cell in this row + # Sets the color for every cell in this row. def color=(color) each_with_index do | cell, index | cell.color = color.is_a?(Array) ? color[index] : color end end - # sets the style for every cell in this row + # Sets the style for every cell in this row. def style=(style) each_with_index do | cell, index | cell.style = style.is_a?(Array) ? style[index] : style end end + # Sets escape_formulas for every cell in this row. This determines whether to treat + # values starting with an equals sign as formulas or as literal strings. + # @param [Array, Boolean] value The value to set. + def escape_formulas=(value) + each_with_index do | cell, index | + cell.escape_formulas = value.is_a?(Array) ? value[index] : value + end + end + # @see height def height=(v) unless v.nil? @@ -146,6 +156,7 @@ module Axlsx # @option options [Array] values # @option options [Array, Symbol] types # @option options [Array, Integer] style + # @option options [Array, Boolean] escape_formulas def array_to_cells(values, options={}) DataTypeValidator.validate :array_to_cells, Array, values types, style, formula_values, escape_formulas, offset = options.delete(:types), options.delete(:style), options.delete(:formula_values), options.delete(:escape_formulas), options.delete(:offset) @@ -153,12 +164,11 @@ module Axlsx values.each_with_index do |value, index| options[:style] = style.is_a?(Array) ? style[index] : style if style options[:type] = types.is_a?(Array) ? types[index] : types if types - options[:escape_formulas] = escape_formulas.is_a?(Array) ? escape_formulas[index] : escape_formulas if escape_formulas + options[:escape_formulas] = escape_formulas.is_a?(Array) ? escape_formulas[index] : escape_formulas unless escape_formulas.nil? options[:formula_value] = formula_values[index] if formula_values.is_a?(Array) self[index + offset.to_i] = Cell.new(self, value, options) end end end - end diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb index dca483dc..f50e6b94 100644 --- a/lib/axlsx/workbook/worksheet/worksheet.rb +++ b/lib/axlsx/workbook/worksheet/worksheet.rb @@ -16,12 +16,15 @@ module Axlsx # @option options [Hash] page_margins A hash containing page margins for this worksheet. @see PageMargins # @option options [Hash] print_options A hash containing print options for this worksheet. @see PrintOptions # @option options [Hash] header_footer A hash containing header/footer options for this worksheet. @see HeaderFooter - # @option options [Boolean] show_gridlines indicates if gridlines should be shown for this sheet. + # @option options [Boolean] show_gridlines Whether gridlines should be shown for this sheet. + # @option options [Boolean] escape_formulas Whether formulas should be escaped by default. Can be overridden at a + # row/cell level. def initialize(wb, options={}) self.workbook = wb @sheet_protection = nil initialize_page_options(options) parse_options options + self.escape_formulas = wb.escape_formulas if @escape_formulas.nil? @workbook.worksheets << self @sheet_id = index + 1 yield self if block_given? @@ -46,6 +49,20 @@ module Axlsx @name ||= "Sheet" + (index+1).to_s end + # Whether to treat values starting with an equals sign as formulas or as literal strings. + # Allowing user-generated data to be interpreted as formulas is a security risk. + # See https://www.owasp.org/index.php/CSV_Injection for details. + # @return [Boolean] + attr_reader :escape_formulas + + # Sets whether to treat values starting with an equals sign as formulas or as literal strings. + # @param [Boolean] value The value to set. + # @return [Boolean] + def escape_formulas=(value) + Axlsx.validate_boolean(value) + @escape_formulas = value + end + # Specifies the visible state of this sheet. Allowed states are # :visible, :hidden or :very_hidden. The default value is :visible. # @@ -413,6 +430,7 @@ module Axlsx # Allowing user generated data to be interpreted as formulas can be dangerous # (see https://www.owasp.org/index.php/CSV_Injection for details). def add_row(values=[], options={}) + options[:escape_formulas] = escape_formulas if options[:escape_formulas].nil? row = Row.new(self, values, options) update_column_info row, options.delete(:widths) yield row if block_given? @@ -835,6 +853,5 @@ module Axlsx return if !auto_filter.range workbook.add_defined_name auto_filter.defined_name, name: '_xlnm._FilterDatabase', local_sheet_id: index, hidden: 1 end - end end diff --git a/test/workbook/tc_workbook.rb b/test/workbook/tc_workbook.rb index f8d0b1f7..a9bd339c 100644 --- a/test/workbook/tc_workbook.rb +++ b/test/workbook/tc_workbook.rb @@ -162,4 +162,40 @@ class TestWorkbook < Test::Unit::TestCase wb_xml = Nokogiri::XML(@wb.to_xml_string) assert_equal sheet.name, wb_xml.xpath('//xmlns:workbook/xmlns:sheets/*[1]/@name').to_s end + + def test_escape_formulas + old = Axlsx::escape_formulas + + Axlsx::escape_formulas = false + p = Axlsx::Package.new + @wb = p.workbook + assert_false @wb.escape_formulas + assert_false @wb.add_worksheet.escape_formulas + assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas + assert @wb.add_worksheet(escape_formulas: true).escape_formulas + + Axlsx::escape_formulas = true + p = Axlsx::Package.new + @wb = p.workbook + assert @wb.escape_formulas + assert @wb.add_worksheet.escape_formulas + assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas + assert @wb.add_worksheet(escape_formulas: true).escape_formulas + + @wb.escape_formulas = false + assert_false @wb.escape_formulas + assert_false @wb.add_worksheet.escape_formulas + assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas + assert @wb.add_worksheet(escape_formulas: true).escape_formulas + + @wb.escape_formulas = true + p = Axlsx::Package.new + @wb = p.workbook + assert @wb.escape_formulas + assert @wb.add_worksheet.escape_formulas + assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas + assert @wb.add_worksheet(escape_formulas: true).escape_formulas + + Axlsx::escape_formulas = old + end end diff --git a/test/workbook/worksheet/tc_row.rb b/test/workbook/worksheet/tc_row.rb index 38b13806..e29895e2 100644 --- a/test/workbook/worksheet/tc_row.rb +++ b/test/workbook/worksheet/tc_row.rb @@ -157,4 +157,30 @@ class TestRow < Test::Unit::TestCase end end + def test_escape_formulas + @ws.escape_formulas = false + @row = @ws.add_row + assert_false @row.add_cell('').escape_formulas + assert_false @row.add_cell('', escape_formulas: false).escape_formulas + assert @row.add_cell('', escape_formulas: true).escape_formulas + + @row = Axlsx::Row.new(@ws) + assert_false @row.add_cell('').escape_formulas + + @ws.escape_formulas = true + @row = @ws.add_row + + assert @row.add_cell('').escape_formulas + assert_false @row.add_cell('', escape_formulas: false).escape_formulas + assert @row.add_cell('', escape_formulas: true).escape_formulas + + @row.escape_formulas = false + assert_equal [false, false, false], @row.cells.map(&:escape_formulas) + + @row.escape_formulas = true + assert_equal [true, true, true], @row.cells.map(&:escape_formulas) + + @row.escape_formulas = [false, true, false] + assert_equal [false, true, false], @row.cells.map(&:escape_formulas) + end end diff --git a/test/workbook/worksheet/tc_worksheet.rb b/test/workbook/worksheet/tc_worksheet.rb index 8e0c5696..d535e4f7 100644 --- a/test/workbook/worksheet/tc_worksheet.rb +++ b/test/workbook/worksheet/tc_worksheet.rb @@ -7,7 +7,6 @@ class TestWorksheet < Test::Unit::TestCase @ws = @wb.add_worksheet end - def test_pn assert_equal(@ws.pn, "worksheets/sheet1.xml") ws = @ws.workbook.add_worksheet @@ -131,10 +130,8 @@ class TestWorksheet < Test::Unit::TestCase assert_equal(header_footer[key], optioned.header_footer.send(key)) end assert_equal(optioned.name, 'bob') - end - # def test_use_gridlines # assert_raise(ArgumentError) { @ws.show_gridlines = -1.1 } # assert_nothing_raised { @ws.show_gridlines = false } @@ -210,7 +207,6 @@ class TestWorksheet < Test::Unit::TestCase @ws.add_row [1, 2, 3, 4] @ws.add_row [1, 2, 3, 4] - assert(@ws.row_breaks.empty?) assert(@ws.col_breaks.empty?) @ws.add_page_break(@ws.rows.last.cells[1]) @@ -218,7 +214,6 @@ class TestWorksheet < Test::Unit::TestCase assert_equal(1, @ws.col_breaks.size) end - def test_drawing assert @ws.drawing == nil @ws.add_chart(Axlsx::Pie3DChart) @@ -442,6 +437,7 @@ class TestWorksheet < Test::Unit::TestCase assert_equal("foo\n\r\nbar", @ws.rows.last.cells.last.value) assert_not_nil(@ws.to_xml_string.index("foo\n\r\nbar")) end + # Make sure the XML for all optional elements (like pageMargins, autoFilter, ...) # is generated in correct order. def test_valid_with_optional_elements @@ -589,7 +585,6 @@ class TestWorksheet < Test::Unit::TestCase assert_equal(other_ws.index, filter_database[1].local_sheet_id) end - def test_sheet_pr_for_auto_filter @ws.auto_filter.range = 'A1:D9' @ws.auto_filter.add_column 0, :filters, :filter_items => [1] @@ -838,7 +833,7 @@ class TestWorksheet < Test::Unit::TestCase sheet.add_border 'B2:D4', style: :medium sheet.add_style 'D2:D4', border: { style: :thin, color: '000000' } end - + wb.apply_styles assert_equal 8, wb.styled_cells.count @@ -884,17 +879,54 @@ class TestWorksheet < Test::Unit::TestCase wb.apply_styles assert_equal 1, wb.styles.style_index.size - + assert_equal( { - type: :xf, - name: "Times New Roman", - sz: 12, - family: 1, + type: :xf, + name: "Times New Roman", + sz: 12, + family: 1, color: "FFFFFF", - }, + }, wb.styles.style_index.values.first ) end + def test_escape_formulas + @wb.escape_formulas = false + @ws = @wb.add_worksheet + assert_false @ws.escape_formulas + assert_false @ws.add_row(['']).cells.first.escape_formulas + assert_false @ws.add_row([''], escape_formulas: false).cells.first.escape_formulas + assert @ws.add_row([''], escape_formulas: true).cells.first.escape_formulas + assert_equal [true, false], @ws.add_row(['', ''], escape_formulas: [true, false]).cells.map(&:escape_formulas) + + @ws = Axlsx::Worksheet.new(@wb) + assert_false @ws.escape_formulas + + @wb.escape_formulas = true + @ws = @wb.add_worksheet + assert @ws.escape_formulas + assert @ws.add_row(['']).cells.first.escape_formulas + assert_false @ws.add_row([''], escape_formulas: false).cells.first.escape_formulas + assert @ws.add_row([''], escape_formulas: true).cells.first.escape_formulas + assert_equal [true, false], @ws.add_row(['', ''], escape_formulas: [true, false]).cells.map(&:escape_formulas) + + @ws = Axlsx::Worksheet.new(@wb) + assert @ws.escape_formulas + + @ws.escape_formulas = false + assert_false @ws.escape_formulas + assert_false @ws.add_row(['']).cells.first.escape_formulas + assert_false @ws.add_row([''], escape_formulas: false).cells.first.escape_formulas + assert @ws.add_row([''], escape_formulas: true).cells.first.escape_formulas + assert_equal [true, false], @ws.add_row(['', ''], escape_formulas: [true, false]).cells.map(&:escape_formulas) + + @ws.escape_formulas = true + assert @ws.escape_formulas + assert @ws.add_row(['']).cells.first.escape_formulas + assert_false @ws.add_row([''], escape_formulas: false).cells.first.escape_formulas + assert @ws.add_row([''], escape_formulas: true).cells.first.escape_formulas + assert_equal [true, false], @ws.add_row(['', ''], escape_formulas: [true, false]).cells.map(&:escape_formulas) + end end -- cgit v1.2.3 From c7c30f2fe05b6557454bc7e866f5091b24d8a975 Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Sat, 1 Apr 2023 09:59:04 +0900 Subject: Fix global setting of escape_formulas --- lib/axlsx.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/axlsx.rb b/lib/axlsx.rb index c3532b72..a21d6dfa 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -207,7 +207,7 @@ module Axlsx # See https://www.owasp.org/index.php/CSV_Injection for details. # @return [Boolean] def self.escape_formulas - @escape_formulas || false + @escape_formulas.nil? ? false : @escape_formulas end # Sets whether to treat values starting with an equals sign as formulas or as literal strings. -- cgit v1.2.3 From 377ad94928c3f76e36d0c2aef05fca5dd13e1aae Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Sat, 1 Apr 2023 17:06:08 +0900 Subject: Additional tests + CHANGELOG --- CHANGELOG.md | 1 + test/tc_axlsx.rb | 13 ++++++++++++- test/workbook/tc_workbook.rb | 4 +--- 3 files changed, 14 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 39002c80..8537ea6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ CHANGELOG --------- - **Unreleased** + - [PR #186](https://github.com/caxlsx/caxlsx/pull/186) - Add `escape_formulas` to global, workbook, worksheet, row and cell levels, and standardize behavior. - Fix bug when calling `worksheet.add_border("A1:B2", nil)` - Change `BorderCreator#initialize` arguments handling - Fix `add_border` to work with singluar cell refs diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index 3903fc42..a4069cfe 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -22,7 +22,6 @@ class TestAxlsx < Test::Unit::TestCase assert_equal false, Axlsx.trust_input end - def test_trust_input_can_be_set_to_true # Class variables like this are not reset between test runs, so we have # to save and restore the original value manually. @@ -140,4 +139,16 @@ class TestAxlsx < Test::Unit::TestCase assert_equal({foo: {bar: true, baz: true}}, Axlsx.hash_deep_merge(h1, h2)) end + def test_escape_formulas + Axlsx.instance_variable_set(:@escape_formulas, nil) + assert_equal false, Axlsx::escape_formulas + + Axlsx::escape_formulas = true + assert_equal true, Axlsx::escape_formulas + + Axlsx::escape_formulas = false + assert_equal false, Axlsx::escape_formulas + + Axlsx.instance_variable_set(:@escape_formulas, nil) + end end diff --git a/test/workbook/tc_workbook.rb b/test/workbook/tc_workbook.rb index a9bd339c..99ed69f4 100644 --- a/test/workbook/tc_workbook.rb +++ b/test/workbook/tc_workbook.rb @@ -164,8 +164,6 @@ class TestWorkbook < Test::Unit::TestCase end def test_escape_formulas - old = Axlsx::escape_formulas - Axlsx::escape_formulas = false p = Axlsx::Package.new @wb = p.workbook @@ -196,6 +194,6 @@ class TestWorkbook < Test::Unit::TestCase assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas assert @wb.add_worksheet(escape_formulas: true).escape_formulas - Axlsx::escape_formulas = old + Axlsx.instance_variable_set(:@escape_formulas, nil) end end -- cgit v1.2.3 From aad14cd003e08fae6fdb29ab675b975c9619b8c3 Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Sat, 1 Apr 2023 20:13:54 +0900 Subject: Add ensure to specs --- test/tc_axlsx.rb | 2 +- test/workbook/tc_workbook.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index a4069cfe..24c233ac 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -148,7 +148,7 @@ class TestAxlsx < Test::Unit::TestCase Axlsx::escape_formulas = false assert_equal false, Axlsx::escape_formulas - + ensure Axlsx.instance_variable_set(:@escape_formulas, nil) end end diff --git a/test/workbook/tc_workbook.rb b/test/workbook/tc_workbook.rb index 99ed69f4..958a1a7b 100644 --- a/test/workbook/tc_workbook.rb +++ b/test/workbook/tc_workbook.rb @@ -193,7 +193,7 @@ class TestWorkbook < Test::Unit::TestCase assert @wb.add_worksheet.escape_formulas assert_false @wb.add_worksheet(escape_formulas: false).escape_formulas assert @wb.add_worksheet(escape_formulas: true).escape_formulas - + ensure Axlsx.instance_variable_set(:@escape_formulas, nil) end end -- cgit v1.2.3 From 98a26ea04e886ce6f712e4e97e6b9d09b4691bb6 Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Sun, 2 Apr 2023 03:11:34 +0900 Subject: `escape_formulas` should handle all [OWASP-designated formula prefixes](https://owasp.org/www-community/attacks/CSV_Injection). --- CHANGELOG.md | 1 + lib/axlsx/workbook/worksheet/cell.rb | 8 ++++++-- test/workbook/worksheet/tc_cell.rb | 33 ++++++++++++++++++++++++++++++++- 3 files changed, 39 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8537ea6e..1ebf252c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ CHANGELOG --------- - **Unreleased** - [PR #186](https://github.com/caxlsx/caxlsx/pull/186) - Add `escape_formulas` to global, workbook, worksheet, row and cell levels, and standardize behavior. + - [PR #186](https://github.com/caxlsx/caxlsx/pull/186) - `escape_formulas` should handle all [OWASP-designated formula prefixes](https://owasp.org/www-community/attacks/CSV_Injection). - Fix bug when calling `worksheet.add_border("A1:B2", nil)` - Change `BorderCreator#initialize` arguments handling - Fix `add_border` to work with singluar cell refs diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 8e55d0c5..f0345705 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -72,6 +72,10 @@ module Axlsx CELL_TYPES = [:date, :time, :float, :integer, :richtext, :string, :boolean, :iso_8601, :text].freeze + # Leading characters that indicate a formula. + # See: https://owasp.org/www-community/attacks/CSV_Injection + FORMULA_PREFIXES = ['-', '=', '+', '@', '%', '|', "\r", "\t"].freeze + # The index of the cellXfs item to be applied to this cell. # @return [Integer] # @see Axlsx::Styles @@ -170,7 +174,7 @@ module Axlsx !is_text_run? && # No inline styles !@value.nil? && # Not nil !@value.empty? && # Not empty - !@value.start_with?(?=) # Not a formula + !@value.start_with?(*FORMULA_PREFIXES) # Not a formula end # The inline font_name property for the cell @@ -368,7 +372,7 @@ module Axlsx def is_formula? return false if escape_formulas - type == :string && @value.to_s.start_with?(?=) + type == :string && @value.to_s.start_with?(*FORMULA_PREFIXES) end def is_array_formula? diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb index bdbfd59d..b8e16404 100644 --- a/test/workbook/worksheet/tc_cell.rb +++ b/test/workbook/worksheet/tc_cell.rb @@ -382,6 +382,37 @@ class TestCell < Test::Unit::TestCase assert(doc.xpath("//t[text()='=IF(2+2=4,4,5)']").any?) end + def test_to_xml_string_numeric_escaped + p = Axlsx::Package.new + ws = p.workbook.add_worksheet do |sheet| + sheet.add_row ["-1", "+2"], escape_formulas: true, types: :text + end + doc = Nokogiri::XML(ws.to_xml_string) + doc.remove_namespaces! + assert(doc.xpath("//t[text()='-1']").any?) + assert(doc.xpath("//t[text()='+2']").any?) + end + + def test_to_xml_string_other_owasp_escaped + p = Axlsx::Package.new + ws = p.workbook.add_worksheet do |sheet| + sheet.add_row [ + "@1", + "%2", + "|3", + "\rfoo", + "\tbar" + ], escape_formulas: true + end + doc = Nokogiri::XML(ws.to_xml_string) + doc.remove_namespaces! + assert(doc.xpath("//t[text()='@1']").any?) + assert(doc.xpath("//t[text()='%2']").any?) + assert(doc.xpath("//t[text()='|3']").any?) + assert(doc.xpath("//t[text()='\nfoo']").any?) + assert(doc.xpath("//t[text()='\tbar']").any?) + end + def test_to_xml_string_formula_escape_array_parameter p = Axlsx::Package.new ws = p.workbook.add_worksheet do |sheet| @@ -414,7 +445,7 @@ class TestCell < Test::Unit::TestCase def test_to_xml_string_text_formula p = Axlsx::Package.new ws = p.workbook.add_worksheet do |sheet| - sheet.add_row ["=1+1", "-1+1"], type: :text + sheet.add_row ["=1+1", "-1+1"], types: :text end doc = Nokogiri::XML(ws.to_xml_string) doc.remove_namespaces! -- cgit v1.2.3 From 467ac5cb9e03ff975929dd5bf473b75f7b6f1b6c Mon Sep 17 00:00:00 2001 From: johnnyshields Date: Mon, 3 Apr 2023 10:01:24 +0900 Subject: Escape array formulas --- lib/axlsx/workbook/worksheet/cell.rb | 5 ++++- test/workbook/worksheet/tc_cell.rb | 13 +++++++++++++ 2 files changed, 17 insertions(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index f0345705..39452c47 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -174,7 +174,8 @@ module Axlsx !is_text_run? && # No inline styles !@value.nil? && # Not nil !@value.empty? && # Not empty - !@value.start_with?(*FORMULA_PREFIXES) # Not a formula + !is_formula? && # Not a formula + !is_array_formula? # Not an array formula end # The inline font_name property for the cell @@ -376,6 +377,8 @@ module Axlsx end def is_array_formula? + return false if escape_formulas + type == :string && @value.to_s.start_with?('{=') && @value.to_s.end_with?('}') end diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb index b8e16404..29f42c4f 100644 --- a/test/workbook/worksheet/tc_cell.rb +++ b/test/workbook/worksheet/tc_cell.rb @@ -318,6 +318,8 @@ class TestCell < Test::Unit::TestCase end def test_plain_string + @c.escape_formulas = false + @c.type = :integer assert_equal(@c.plain_string?, false) @@ -334,6 +336,17 @@ class TestCell < Test::Unit::TestCase @c.value = '=sum' assert_equal(@c.plain_string?, false) + @c.value = '{=sum}' + assert_equal(@c.plain_string?, false) + + @c.escape_formulas = true + + @c.value = '=sum' + assert_equal(@c.plain_string?, true) + + @c.value = '{=sum}' + assert_equal(@c.plain_string?, true) + @c.value = 'plain string' @c.font_name = 'Arial' assert_equal(@c.plain_string?, false) -- cgit v1.2.3 From 63b7e742e4146c1d174413ff2e44d3b6c20b83cf Mon Sep 17 00:00:00 2001 From: Johnny Shields Date: Thu, 13 Apr 2023 00:06:56 +0900 Subject: Update row.rb --- lib/axlsx/workbook/worksheet/row.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/axlsx/workbook/worksheet/row.rb b/lib/axlsx/workbook/worksheet/row.rb index d8c73060..119cd10b 100644 --- a/lib/axlsx/workbook/worksheet/row.rb +++ b/lib/axlsx/workbook/worksheet/row.rb @@ -122,7 +122,7 @@ module Axlsx # values starting with an equals sign as formulas or as literal strings. # @param [Array, Boolean] value The value to set. def escape_formulas=(value) - each_with_index do | cell, index | + each_with_index do |cell, index| cell.escape_formulas = value.is_a?(Array) ? value[index] : value end end -- cgit v1.2.3