Skip to content

Commit 3338621

Browse files
committed
Use a temporary file for storing unencrypted files while editing
When we're editing the contents of encrypted files, we should use the `Tempfile` class because it creates temporary files with restrictive permissions. This prevents other users on the same system from reading the contents of those files while the user is editing them. [CVE-2023-38037]
1 parent 5f6c404 commit 3338621

File tree

3 files changed

+26
-17
lines changed

3 files changed

+26
-17
lines changed

activesupport/lib/active_support/encrypted_file.rb

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
# frozen_string_literal: true
22

33
require "pathname"
4-
require "tmpdir"
4+
require "tempfile"
55
require "active_support/message_encryptor"
66

77
module ActiveSupport
@@ -87,17 +87,16 @@ def change(&block)
8787

8888
private
8989
def writing(contents)
90-
tmp_file = "#{Process.pid}.#{content_path.basename.to_s.chomp('.enc')}"
91-
tmp_path = Pathname.new File.join(Dir.tmpdir, tmp_file)
92-
tmp_path.binwrite contents
90+
Tempfile.create(["", "-" + content_path.basename.to_s.chomp(".enc")]) do |tmp_file|
91+
tmp_path = Pathname.new(tmp_file)
92+
tmp_path.binwrite contents
9393

94-
yield tmp_path
94+
yield tmp_path
9595

96-
updated_contents = tmp_path.binread
96+
updated_contents = tmp_path.binread
9797

98-
write(updated_contents) if updated_contents != contents
99-
ensure
100-
FileUtils.rm(tmp_path) if tmp_path&.exist?
98+
write(updated_contents) if updated_contents != contents
99+
end
101100
end
102101

103102

activesupport/test/encrypted_file_test.rb

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,14 @@ class EncryptedFileTest < ActiveSupport::TestCase
5252
assert_equal "#{@content} and went by the lake", @encrypted_file.read
5353
end
5454

55+
test "change sets restricted permissions" do
56+
@encrypted_file.write(@content)
57+
@encrypted_file.change do |file|
58+
assert_predicate file, :owned?
59+
assert_equal "100600", file.stat.mode.to_s(8), "Incorrect mode for #{file}"
60+
end
61+
end
62+
5563
test "raise MissingKeyError when key is missing" do
5664
assert_raise ActiveSupport::EncryptedFile::MissingKeyError do
5765
encrypted_file(@content_path, key_path: "", env_key: "").read

railties/lib/rails/secrets.rb

Lines changed: 10 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
# frozen_string_literal: true
22

33
require "yaml"
4+
require "tempfile"
45
require "active_support/message_encryptor"
56

67
module Rails
@@ -87,17 +88,18 @@ def preprocess(path)
8788
end
8889

8990
def writing(contents)
90-
tmp_file = "#{File.basename(path)}.#{Process.pid}"
91-
tmp_path = File.join(Dir.tmpdir, tmp_file)
92-
IO.binwrite(tmp_path, contents)
91+
file_name = "#{File.basename(path)}.#{Process.pid}"
9392

94-
yield tmp_path
93+
Tempfile.create(["", "-" + file_name]) do |tmp_file|
94+
tmp_path = Pathname.new(tmp_file)
95+
tmp_path.binwrite contents
9596

96-
updated_contents = IO.binread(tmp_path)
97+
yield tmp_path
9798

98-
write(updated_contents) if updated_contents != contents
99-
ensure
100-
FileUtils.rm(tmp_path) if File.exist?(tmp_path)
99+
updated_contents = tmp_path.binread
100+
101+
write(updated_contents) if updated_contents != contents
102+
end
101103
end
102104

103105
def encryptor

0 commit comments

Comments
 (0)