Skip to content

Commit

Permalink
Merge branch 'security-issue-65333-csv-injection' into jansa
Browse files Browse the repository at this point in the history
(cherry picked from commit 78607a3fa64c6de92d1d0d7fc87f07e1eb68a54c)

https://bugzilla.redhat.com/show_bug.cgi?id=1847796
  • Loading branch information
Fryguy authored and simaishi committed Jul 9, 2020
1 parent 615a8a0 commit f0d237e
Show file tree
Hide file tree
Showing 2 changed files with 81 additions and 4 deletions.
32 changes: 32 additions & 0 deletions lib/patches/ruport_patch.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,35 @@ def sort_rows_by(col_names = nil, options = {}, &block)
end
end
end

module Ruport

# Handles preventing CSV injection attacks by adding an apostrophe to all
# fields that could potentially be a formula that executes a function.
#
# This targets values that both:
#
# - Start with '=', '+', '-', or '@' characters
# - Include a '(' or '!' in them
#
# Either of which could be executing a particular function, but still retains
# some features of simple arithmatic operations of speadsheets if users
# happen to be using them, as well as not mucking with "negative numbers" for
# just using the raw CSV in a scripting language like Ruby/Python.
#
class Formatter::SafeCSV < Formatter::CSV
renders :csv, :for => [ Controller::Row, Controller::Table,
Controller::Group, Controller::Grouping ]

def build_table_body
data.each do |row|
row_data = row.map do |column_value|
column_value.insert(0, "'") if column_value =~ /^\s*[@=+-]/ && column_value =~ /[(!\/]/
column_value
end

csv_writer << row_data
end
end
end
end
53 changes: 49 additions & 4 deletions spec/models/miq_report/formatters/csv_spec.rb
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
describe MiqReport::Formatters::Csv do
describe "#to_csv" do
let(:col_options) { nil }
let(:name_1) { "vmware" }
let(:name_2) { "vmware1" }
let(:csv_name_1) { "vmware" }
let(:csv_name_2) { "vmware1" }
let(:table_data) do
[{"id" => 5, "name" => "vmware", "base_name" => "XXX", "file_version" => "11", "size" => "33", "contents_available" => true, "permissions" => nil, "updated_on" => nil, "mtime" => nil},
{"id" => 1, "name" => "vmware1", "base_name" => "YYY", "file_version" => "22", "size" => "44", "contents_available" => true, "permissions" => nil, "updated_on" => nil, "mtime" => nil}]
[{"id" => 5, "name" => name_1, "base_name" => "XXX", "file_version" => "11", "size" => "33", "contents_available" => true, "permissions" => nil, "updated_on" => nil, "mtime" => nil},
{"id" => 1, "name" => name_2, "base_name" => "YYY", "file_version" => "22", "size" => "44", "contents_available" => true, "permissions" => nil, "updated_on" => nil, "mtime" => nil}]
end

let(:miq_report_filesystem) do
Expand All @@ -15,7 +19,11 @@
end

let(:csv_output) do
"\"Name\",\"File Name\",\"File Version\",\"Size\",\"Contents Available\",\"Permissions\",\"Collected On\",\"Last Modified\"\nvmware,XXX,11,33,true,,,\nvmware1,YYY,22,44,true,,,\n"
<<~CSV
"Name","File Name","File Version","Size","Contents Available","Permissions","Collected On","Last Modified"
#{csv_name_1},XXX,11,33,true,,,
#{csv_name_2},YYY,22,44,true,,,
CSV
end

it "generates csv report" do
Expand All @@ -28,12 +36,49 @@
end

let(:csv_output) do
"\"File Name\",\"Size\",\"Contents Available\",\"Permissions\",\"Collected On\",\"Last Modified\"\nXXX,33,true,,,\nYYY,44,true,,,\n"
<<~CSV
"File Name","Size","Contents Available","Permissions","Collected On","Last Modified"
XXX,33,true,,,
YYY,44,true,,,
CSV
end

it "hides columns in csv report" do
expect(miq_report_filesystem.to_csv).to eq(csv_output)
end
end

context "with spreadsheet formulas injected in the contents" do
SPREADSHEET_FORMULA_VALUE_PREFIXES = %w[= + - @]

SPREADSHEET_FORMULA_VALUE_PREFIXES.each do |prefix|
context "first column starts with '#{prefix}' with '!' present" do
let(:name_1) { "#{prefix}cmd|' /C notepad'!'B1'" }
let(:csv_name_1) { "'#{prefix}cmd|' /C notepad'!'B1'" }

it "escapes the column data" do
expect(miq_report_filesystem.to_csv).to eq(csv_output)
end
end

context "first column starts with '#{prefix}' with '(' present" do
let(:name_1) { %Q{#{prefix}HYPERLINK("example.com/vm/B1","Link to B1")} }
let(:csv_name_1) { %Q{"'#{prefix}HYPERLINK(""example.com/vm/B1"",""Link to B1"")"} }

it "escapes the column data" do
expect(miq_report_filesystem.to_csv).to eq(csv_output)
end
end

context "first column starts with '#{prefix}' without '!' or '(' present" do
let(:name_1) { "#{prefix}B1" }
let(:csv_name_1) { "#{prefix}B1" }

it "does not escape column data" do
expect(miq_report_filesystem.to_csv).to eq(csv_output)
end
end
end
end
end
end

0 comments on commit f0d237e

Please sign in to comment.