From 0746815b75296bcf65d49a66f0dca1427ac65f3e Mon Sep 17 00:00:00 2001
From: johnnyshields <johnny.shields@gmail.com>
Date: Fri, 31 Mar 2023 04:40:41 +0900
Subject: Add settings for escape_formulas at global, workbook, worksheet, row
 and cell levels.

---
 test/workbook/tc_workbook.rb            | 36 ++++++++++++++++++++
 test/workbook/worksheet/tc_row.rb       | 26 +++++++++++++++
 test/workbook/worksheet/tc_worksheet.rb | 58 +++++++++++++++++++++++++--------
 3 files changed, 107 insertions(+), 13 deletions(-)

(limited to 'test')

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 377ad94928c3f76e36d0c2aef05fca5dd13e1aae Mon Sep 17 00:00:00 2001
From: johnnyshields <johnny.shields@gmail.com>
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(-)

(limited to 'test')

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 <johnny.shields@gmail.com>
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(-)

(limited to 'test')

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 <johnny.shields@gmail.com>
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(-)

(limited to 'test')

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 <johnny.shields@gmail.com>
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(-)

(limited to 'test')

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