Skip to content

Commit

Permalink
Use a temporary file for storing unencrypted files while editing
Browse files Browse the repository at this point in the history
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]
  • Loading branch information
tenderlove committed Aug 22, 2023
1 parent 7d949d7 commit c85cc66
Show file tree
Hide file tree
Showing 3 changed files with 26 additions and 17 deletions.
17 changes: 8 additions & 9 deletions activesupport/lib/active_support/encrypted_file.rb
@@ -1,7 +1,7 @@
# frozen_string_literal: true

require "pathname"
require "tmpdir"
require "tempfile"
require "active_support/message_encryptor"

module ActiveSupport
Expand Down Expand Up @@ -69,17 +69,16 @@ def change(&block)

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

yield tmp_path
yield tmp_path

updated_contents = tmp_path.binread
updated_contents = tmp_path.binread

write(updated_contents) if updated_contents != contents
ensure
FileUtils.rm(tmp_path) if tmp_path&.exist?
write(updated_contents) if updated_contents != contents
end
end


Expand Down
8 changes: 8 additions & 0 deletions activesupport/test/encrypted_file_test.rb
Expand Up @@ -49,6 +49,14 @@ class EncryptedFileTest < ActiveSupport::TestCase
assert_equal "#{@content} and went by the lake", @encrypted_file.read
end

test "change sets restricted permissions" do
@encrypted_file.write(@content)
@encrypted_file.change do |file|
assert_predicate file, :owned?
assert_equal "100600", file.stat.mode.to_s(8), "Incorrect mode for #{file}"
end
end

test "raise MissingKeyError when key is missing" do
assert_raise ActiveSupport::EncryptedFile::MissingKeyError do
encrypted_file(@content_path, key_path: "", env_key: "").read
Expand Down
18 changes: 10 additions & 8 deletions railties/lib/rails/secrets.rb
@@ -1,6 +1,7 @@
# frozen_string_literal: true

require "yaml"
require "tempfile"
require "active_support/message_encryptor"

module Rails
Expand Down Expand Up @@ -87,17 +88,18 @@ def preprocess(path)
end

def writing(contents)
tmp_file = "#{File.basename(path)}.#{Process.pid}"
tmp_path = File.join(Dir.tmpdir, tmp_file)
IO.binwrite(tmp_path, contents)
file_name = "#{File.basename(path)}.#{Process.pid}"

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

updated_contents = IO.binread(tmp_path)
yield tmp_path

write(updated_contents) if updated_contents != contents
ensure
FileUtils.rm(tmp_path) if File.exist?(tmp_path)
updated_contents = tmp_path.binread

write(updated_contents) if updated_contents != contents
end
end

def encryptor
Expand Down

0 comments on commit c85cc66

Please sign in to comment.