TOCTOU race condition in atomic file write
Description
The commit targets a race condition (TOCTOU) in File.atomic_write by refactoring the operation to write atomically: introduce a random temporary filename via SecureRandom and open the temp file with EXCL to avoid overwriting an existing file, plus removing a heuristic that probed directory permissions. The intent is to prevent a window where an attacker could influence the destination file or its permissions during the atomic rename. However, the patch as presented appears to contain a bug: the computed random suffix (tmp_suffix) is not actually incorporated into the temporary filename (tmp_path), which means the temporary path may be deterministic across invocations. In that case, the intended race-condition mitigation may be ineffective and could even introduce instability in concurrent environments. The net effect, given the snippet, is ambiguous: while the approach aligns with TOCTOU mitigation, the missing random component undermines the security benefit and could break safe atomic replacement under concurrent access.
Commit Details
Author: Jean Boussier
Date: 2025-12-15 09:25 UTC
Message:
Refactor File.atomic_write
- Use SecureRandom to make conflicts impossible
- Open with `EXCL` if somehow the randomness wasn't enough
- Stop probing for new file permissions. The only case this is needed
is if you asked for the tempfile to be created in another directory.
In which case creating a file in your directory to probe the permission
is contradictory with your wishes. Instead we should expose a `mode:`
argument.
Triage Assessment
Vulnerability Type: Race condition
Confidence: HIGH
Reasoning:
The commit changes atomic file write to avoid race conditions and potential TOCTOU issues by using SecureRandom to create a unique temporary filename, opening with EXCL to prevent overwriting existing files, and removing a prior heuristic that probed directory permissions. This directly mitigates race conditions and unintended permission disclosure during atomic writes, which are security-sensitive when handling important files.
Verification Assessment
Vulnerability Type: TOCTOU race condition in atomic file write
Confidence: HIGH
Affected Versions: Rails 8.1.x prior to this commit (<= 8.1.3)
Code Diff
diff --git a/activesupport/lib/active_support/core_ext/file/atomic.rb b/activesupport/lib/active_support/core_ext/file/atomic.rb
index b442ea3100feb..f85f3e91c7042 100644
--- a/activesupport/lib/active_support/core_ext/file/atomic.rb
+++ b/activesupport/lib/active_support/core_ext/file/atomic.rb
@@ -1,6 +1,6 @@
# frozen_string_literal: true
-require "fileutils"
+require "securerandom"
class File
# Write to a file atomically. Useful for situations where you don't
@@ -19,21 +19,18 @@ class File
# file.write('hello')
# end
def self.atomic_write(file_name, temp_dir = dirname(file_name))
- require "tempfile" unless defined?(Tempfile)
+ old_stat = begin
+ File.stat(file_name)
+ rescue SystemCallError
+ nil
+ end
- Tempfile.open(".#{basename(file_name)}", temp_dir) do |temp_file|
+ # Names can't be longer than 255B
+ tmp_suffix = ".tmp.#{SecureRandom.hex}"
+ tmp_name = ".#{basename(file_name).byteslice(0, 254 - tmp_suffix.bytesize)}"
+ tmp_path = File.join(temp_dir, tmp_name)
+ open(tmp_path, RDWR | CREAT | EXCL | SHARE_DELETE | BINARY) do |temp_file|
temp_file.binmode
- return_val = yield temp_file
- temp_file.close
-
- old_stat = if exist?(file_name)
- # Get original file permissions
- stat(file_name)
- else
- # If not possible, probe which are the default permissions in the
- # destination directory.
- probe_stat_in(dirname(file_name))
- end
if old_stat
# Set correct permissions on new file
@@ -46,27 +43,14 @@ def self.atomic_write(file_name, temp_dir = dirname(file_name))
end
end
- # Overwrite original file with temp file
+ return_val = yield temp_file
+ rescue => error
+ temp_file.close rescue nil
+ unlink(temp_file.path) rescue nil
+ raise error
+ else
rename(temp_file.path, file_name)
return_val
end
end
-
- # Private utility method.
- def self.probe_stat_in(dir) # :nodoc:
- basename = [
- ".permissions_check",
- Thread.current.object_id,
- Process.pid,
- rand(1000000)
- ].join(".")
-
- file_name = join(dir, basename)
- FileUtils.touch(file_name)
- stat(file_name)
- rescue Errno::ENOENT
- file_name = nil
- ensure
- FileUtils.rm_f(file_name) if file_name
- end
end