Skip to content

Commit 4da3e2b

Browse files
committed
#357: fix TOCTOU race in durable_place by opening file before checking
1 parent 5a12d9d commit 4da3e2b

2 files changed

Lines changed: 35 additions & 17 deletions

File tree

lib/baza-rb.rb

Lines changed: 21 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -298,26 +298,30 @@ def durable_place(pname, file)
298298
raise(RuntimeError, 'The "pname" of the durable is nil') if pname.nil?
299299
raise(RuntimeError, 'The "pname" of the durable may not be empty') if pname.empty?
300300
raise(RuntimeError, 'The "file" of the durable is nil') if file.nil?
301-
raise(RuntimeError, "The file '#{file}' is absent") unless File.exist?(file)
302-
if File.size(file) > 1024
303-
raise(
304-
RuntimeError,
305-
"The file '#{file}' is too big (#{File.size(file)} bytes) for durable_place(), use durable_save() instead"
306-
)
307-
end
308301
id = nil
309-
elapsed(@loog, level: Logger::INFO) do
310-
id = post(
311-
home.append('durable-place'),
312-
{
313-
'pname' => pname,
314-
'file' => File.basename(file),
315-
'zip' => File.open(file, 'rb')
316-
}
317-
).headers['X-Zerocracy-DurableId'].to_i
318-
throw(:"Durable ##{id} (#{file}, #{File.size(file)} bytes) placed for job \"#{pname}\" at #{@host}")
302+
File.open(file, 'rb') do |f|
303+
raise(RuntimeError, "The file '#{file}' is absent") unless f.stat.file?
304+
if f.size > 1024
305+
raise(
306+
RuntimeError,
307+
"The file '#{file}' is too big (#{f.size} bytes) for durable_place(), use durable_save() instead"
308+
)
309+
end
310+
elapsed(@loog, level: Logger::INFO) do
311+
id = post(
312+
home.append('durable-place'),
313+
{
314+
'pname' => pname,
315+
'file' => File.basename(file),
316+
'zip' => f
317+
}
318+
).headers['X-Zerocracy-DurableId'].to_i
319+
throw(:"Durable ##{id} (#{file}, #{f.size} bytes) placed for job \"#{pname}\" at #{@host}")
320+
end
319321
end
320322
id
323+
rescue Errno::ENOENT
324+
raise(RuntimeError, "The file '#{file}' is absent")
321325
end
322326

323327
# Save a single durable from local file to server.

test/test_baza_edge.rb

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -49,6 +49,20 @@ def test_durable_place
4949
end
5050
end
5151

52+
def test_durable_place_rejects_symlink_to_non_regular_file
53+
WebMock.disable_net_connect!
54+
Dir.mktmpdir do |dir|
55+
link = File.join(dir, 'target.bin')
56+
File.symlink('/dev/null', link)
57+
baza = fake_baza(compress: false)
58+
error =
59+
assert_raises(RuntimeError) do
60+
baza.durable_place('simple', link)
61+
end
62+
assert_includes(error.message, 'absent')
63+
end
64+
end
65+
5266
def test_real_http
5367
WebMock.enable_net_connect!
5468
assert_equal(

0 commit comments

Comments
 (0)