diff options
| -rw-r--r-- | CHANGELOG.md | 2 | ||||
| -rw-r--r-- | README.md | 20 | ||||
| -rw-r--r-- | examples/escape_formula_example.md | 20 | ||||
| -rw-r--r-- | lib/axlsx.rb | 15 | ||||
| -rw-r--r-- | lib/axlsx/workbook/workbook.rb | 15 | ||||
| -rw-r--r-- | lib/axlsx/workbook/worksheet/cell.rb | 37 | ||||
| -rw-r--r-- | lib/axlsx/workbook/worksheet/row.rb | 13 | ||||
| -rw-r--r-- | lib/axlsx/workbook/worksheet/worksheet.rb | 18 | ||||
| -rw-r--r-- | test/tc_axlsx.rb | 13 | ||||
| -rw-r--r-- | test/workbook/tc_workbook.rb | 34 | ||||
| -rw-r--r-- | test/workbook/worksheet/tc_cell.rb | 46 | ||||
| -rw-r--r-- | test/workbook/worksheet/tc_row.rb | 27 | ||||
| -rw-r--r-- | test/workbook/worksheet/tc_worksheet.rb | 38 |
13 files changed, 279 insertions, 19 deletions
diff --git a/CHANGELOG.md b/CHANGELOG.md index da888bff..4e6fa749 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,8 @@ 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 @@ -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 2ee8a1d8..9dba9144 100644 --- a/lib/axlsx.rb +++ b/lib/axlsx.rb @@ -200,4 +200,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.nil? ? false : @escape_formulas + 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 e1c7fb05..71a3c375 100644 --- a/lib/axlsx/workbook/workbook.rb +++ b/lib/axlsx/workbook/workbook.rb @@ -57,6 +57,7 @@ module Axlsx require 'axlsx/workbook/worksheet/sheet_format_pr.rb' require 'axlsx/workbook/worksheet/pane.rb' require 'axlsx/workbook/worksheet/selection.rb' + # 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. # @@ -239,6 +240,7 @@ module Axlsx @bold_font_multiplier = BOLD_FONT_MULTIPLIER @font_scale_divisor = FONT_SCALE_DIVISOR + 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 @@ -258,6 +260,19 @@ module Axlsx # @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 diff --git a/lib/axlsx/workbook/worksheet/cell.rb b/lib/axlsx/workbook/worksheet/cell.rb index 364de1e9..5e2661c3 100644 --- a/lib/axlsx/workbook/worksheet/cell.rb +++ b/lib/axlsx/workbook/worksheet/cell.rb @@ -28,10 +28,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 @@ -40,15 +39,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? @@ -73,6 +70,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 @@ -132,16 +133,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. @@ -170,7 +172,8 @@ module Axlsx !is_text_run? && # No inline styles [email protected]? && # Not nil [email protected]? && # Not empty - [email protected]_with?(?=) # Not a formula + !is_formula? && # Not a formula + !is_array_formula? # Not an array formula end # The inline font_name property for the cell @@ -384,10 +387,12 @@ 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? + return false if escape_formulas + type == :string && @value.to_s.start_with?('{=') && @value.to_s.end_with?('}') end diff --git a/lib/axlsx/workbook/worksheet/row.rb b/lib/axlsx/workbook/worksheet/row.rb index 97a7adab..119cd10b 100644 --- a/lib/axlsx/workbook/worksheet/row.rb +++ b/lib/axlsx/workbook/worksheet/row.rb @@ -23,6 +23,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 @@ -117,6 +118,15 @@ module Axlsx 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? @@ -145,6 +155,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) @@ -152,7 +163,7 @@ 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) diff --git a/lib/axlsx/workbook/worksheet/worksheet.rb b/lib/axlsx/workbook/worksheet/worksheet.rb index 3333c3f4..38fb5254 100644 --- a/lib/axlsx/workbook/worksheet/worksheet.rb +++ b/lib/axlsx/workbook/worksheet/worksheet.rb @@ -14,11 +14,14 @@ module Axlsx # @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 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? @@ -43,6 +46,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. # @@ -410,6 +427,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? diff --git a/test/tc_axlsx.rb b/test/tc_axlsx.rb index c214f8e2..73fdd9ab 100644 --- a/test/tc_axlsx.rb +++ b/test/tc_axlsx.rb @@ -139,4 +139,17 @@ class TestAxlsx < Test::Unit::TestCase assert_equal({ foo: { baz: true } }, h1.merge(h2)) 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 + 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 c085ed91..443dd392 100644 --- a/test/workbook/tc_workbook.rb +++ b/test/workbook/tc_workbook.rb @@ -161,4 +161,38 @@ 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 + 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 + ensure + Axlsx.instance_variable_set(:@escape_formulas, nil) + end end diff --git a/test/workbook/worksheet/tc_cell.rb b/test/workbook/worksheet/tc_cell.rb index f5264115..cb878c12 100644 --- a/test/workbook/worksheet/tc_cell.rb +++ b/test/workbook/worksheet/tc_cell.rb @@ -317,6 +317,8 @@ class TestCell < Test::Unit::TestCase end def test_plain_string + @c.escape_formulas = false + @c.type = :integer assert_equal(@c.plain_string?, false) @@ -333,6 +335,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) @@ -381,6 +394,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| @@ -413,7 +457,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! diff --git a/test/workbook/worksheet/tc_row.rb b/test/workbook/worksheet/tc_row.rb index 29c4415b..cf8fa7c3 100644 --- a/test/workbook/worksheet/tc_row.rb +++ b/test/workbook/worksheet/tc_row.rb @@ -153,4 +153,31 @@ class TestRow < Test::Unit::TestCase assert_equal(c.value, index < offset ? nil : values[index - offset]) 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 f6c65fe4..d83cdf91 100644 --- a/test/workbook/worksheet/tc_worksheet.rb +++ b/test/workbook/worksheet/tc_worksheet.rb @@ -891,4 +891,42 @@ class TestWorksheet < Test::Unit::TestCase 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 |
