Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 17 additions & 1 deletion lib/business/calendar.rb
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,11 @@ module Business
class Calendar
VALID_KEYS = %w[holidays working_days extra_working_dates].freeze

# Calendar names are interpolated into a filesystem path, so restrict them to
# plain identifiers: anything containing path separators, "..", or an absolute
# path would otherwise load a file from outside the configured load_paths.
CALENDAR_NAME_FORMAT = /\A[a-zA-Z0-9_-]+\z/

class << self
attr_accessor :load_paths
end
Expand Down Expand Up @@ -38,14 +43,25 @@ def self.find_calendar_data(calendar_name)
if path.is_a?(Hash)
break path[calendar_name] if path[calendar_name]
else
calendar_path = Pathname.new(path).join("#{calendar_name}.yml")
calendar_path = calendar_path_for(path, calendar_name)
next unless calendar_path.exist?

break YAML.safe_load(calendar_path.read, permitted_classes: [Date])
end
end
end

# Only applied to directory load_paths; hash load_paths are a plain key
# lookup and cannot escape anywhere, so their keys stay unrestricted.
def self.calendar_path_for(directory, calendar_name)
unless calendar_name.to_s.match?(CALENDAR_NAME_FORMAT)
raise ArgumentError, "invalid calendar name: #{calendar_name.inspect}"
end

Pathname.new(directory).join("#{calendar_name}.yml")
end
private_class_method :calendar_path_for

@lock = Mutex.new
def self.load_cached(calendar)
@lock.synchronize do
Expand Down
66 changes: 66 additions & 0 deletions spec/business/calendar_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

require "business/calendar"
require "time"
require "fileutils"

RSpec.configure do |config|
config.mock_with(:rspec) { |mocks| mocks.verify_partial_doubles = true }
Expand Down Expand Up @@ -57,6 +58,71 @@
specify { expect { load_calendar }.to raise_error(/No such calendar/) }
end

context "when given a calendar name that escapes the load path" do
before do
outside = File.join(File.dirname(__FILE__), "../fixtures")
FileUtils.mkdir_p(File.join(outside, "outside"))
File.write(
File.join(outside, "outside", "escaped.yml"),
{ "working_days" => ["monday"] }.to_yaml,
)
end

after do
FileUtils.rm_rf(File.join(File.dirname(__FILE__), "../fixtures", "outside"))
end

context "with a relative traversal" do
let(:calendar) { "../outside/escaped" }

specify do
expect { load_calendar }.
to raise_error(ArgumentError, /invalid calendar name/)
end
end

context "with an absolute path" do
let(:calendar) do
File.join(File.dirname(__FILE__), "../fixtures", "outside", "escaped")
end

specify do
expect { load_calendar }.
to raise_error(ArgumentError, /invalid calendar name/)
end
end

context "with a bare path separator" do
let(:calendar) { "outside/escaped" }

specify do
expect { load_calendar }.
to raise_error(ArgumentError, /invalid calendar name/)
end
end

context "with a name that is only dots" do
let(:calendar) { ".." }

specify do
expect { load_calendar }.
to raise_error(ArgumentError, /invalid calendar name/)
end
end

context "when the escaping name is a key in a hash load path" do
before do
described_class.load_paths = [{ "../outside/escaped" => dummy_calendar }]
end

let(:calendar) { "../outside/escaped" }

it "still resolves, since hash lookups touch no filesystem" do
expect(load_calendar).to be_a described_class
end
end
end

context "when given a calendar that has invalid keys" do
let(:calendar) { "invalid-keys" }

Expand Down
Loading