Skip to content

Commit

Permalink
Backport to 3-0 stable branch protect against CSV Injection (#8161) (#…
Browse files Browse the repository at this point in the history
…8167)

protect against CSV Injection

Read more here https://owasp.org/www-community/attacks/CSV_Injection
  • Loading branch information
mgrunberg committed Dec 11, 2023
1 parent 58b31b7 commit 7af735c
Show file tree
Hide file tree
Showing 2 changed files with 71 additions and 2 deletions.
25 changes: 23 additions & 2 deletions lib/active_admin/csv_builder.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def build(controller, csv)
csv << bom if bom

if column_names
csv << CSV.generate_line(columns.map { |c| encode c.name, options }, **csv_options)
csv << CSV.generate_line(columns.map { |c| sanitize(encode(c.name, options)) }, **csv_options)
end

controller.send(:in_paginated_batches) do |resource|
Expand All @@ -70,7 +70,7 @@ def exec_columns(view_context = nil)

def build_row(resource, columns, options)
columns.map do |column|
encode call_method_or_proc_on(resource, column.data), options
sanitize(encode(call_method_or_proc_on(resource, column.data), options))
end
end

Expand All @@ -86,6 +86,10 @@ def encode(content, options)
end
end

def sanitize(content)
Sanitizer.sanitize(content)
end

def method_missing(method, *args, &block)
if @view_context.respond_to? method
@view_context.public_send method, *args, &block
Expand Down Expand Up @@ -120,4 +124,21 @@ def column_transitive_options
@column_transitive_options ||= @options.slice(*COLUMN_TRANSITIVE_OPTIONS)
end
end

# Prevents CSV Injection according to https://owasp.org/www-community/attacks/CSV_Injection
module Sanitizer
extend self

ATTACK_CHARACTERS = ["=", "+", "-", "@", "\t", "\r"].freeze

def sanitize(value)
return "'#{value}" if require_sanitization?(value)

value
end

def require_sanitization?(value)
value.is_a?(String) && value.starts_with?(*ATTACK_CHARACTERS)
end
end
end
48 changes: 48 additions & 0 deletions spec/unit/csv_builder_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -277,4 +277,52 @@ def view_context
end
end
end

context "csv injection" do
let(:dummy_controller) do
class DummyController
def in_paginated_batches(&block)
Post.all.each(&block)
end

def view_context
MethodOrProcHelper
end
end
DummyController.new
end

let(:builder) do
ActiveAdmin::CSVBuilder.new do
column(:id)
column(:title)
end
end

["=", "+", "-", "@", "\t", "\r"].each do |char|
it "prepends a single quote when column starts with a #{char} character" do
attack = "#{char}1+2"

escaped_attack = "'#{attack}"
escaped_attack = "\"#{escaped_attack}\"" if char == "\r"

post = Post.create!(title: attack)
receiver = []
builder.build dummy_controller, receiver
line = receiver.last
expect(line).to eq "#{post.id},#{escaped_attack}\n"
end

it "accounts for the field separator when character #{char} is used to inject a formula" do
attack = "#{char}1+2'\" ;,#{char}1+2"
escaped_attack = "\"'#{attack.gsub('"', '""')}\""

post = Post.create!(title: attack)
receiver = []
builder.build dummy_controller, receiver
line = receiver.last
expect(line).to eq "#{post.id},#{escaped_attack}\n"
end
end
end
end

0 comments on commit 7af735c

Please sign in to comment.