Skip to content

Commit

Permalink
File.mkdir_p: Fix TOCTOU issue & handle errors from parent mkdir
Browse files Browse the repository at this point in the history
Fix TOCTOU, meaning :file.make_dir returning {:error, :eexists} is
now tested without having to trigger a race-condition.
Also change dir?() returning false from returning :eexist to :enotdir

And handling errors from parent directory creation allows to avoid
non-sensical errors like File.mkdir_p returning :enoent instead of :eacces
  • Loading branch information
lanodan committed Jan 30, 2025
1 parent 965ee9b commit 5c16af6
Show file tree
Hide file tree
Showing 2 changed files with 36 additions and 15 deletions.
29 changes: 14 additions & 15 deletions lib/elixir/lib/file.ex
Original file line number Diff line number Diff line change
Expand Up @@ -310,24 +310,23 @@ defmodule File do
end

defp do_mkdir_p(path) do
if dir?(path) do
parent = Path.dirname(path)

if parent == path do
:ok
else
parent = Path.dirname(path)

if parent == path do
# Protect against infinite loop
{:error, :einval}
else
_ = do_mkdir_p(parent)

case :file.make_dir(path) do
{:error, :eexist} = error ->
if dir?(path), do: :ok, else: error
case do_mkdir_p(parent) do
:ok ->
case :file.make_dir(path) do
{:error, :eexist} ->
if dir?(path), do: :ok, else: {:error, :enotdir}

other ->
other
end

other ->
other
end
e ->
e
end
end
end
Expand Down
22 changes: 22 additions & 0 deletions lib/elixir/test/elixir/file_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -1146,6 +1146,28 @@ defmodule FileTest do
File.mkdir_p!(invalid)
end
end

@tag :unix
test "mkdir_p with non-accessible parent directory" do
fixture = tmp_path("tmp_test_parent")

try do
refute File.exists?(fixture)
assert File.mkdir_p!(fixture) == :ok
%File.Stat{mode: orig_mode} = File.stat!(fixture)
assert File.chmod!(fixture, 0o000) == :ok

child = Path.join(fixture, "child")
refute File.exists?(child)

assert File.mkdir_p(child) == {:error, :eacces}
refute File.exists?(child)

assert File.chmod!(fixture, orig_mode) == :ok
after
File.rm_rf(fixture)
end
end
end

describe "rm" do
Expand Down

0 comments on commit 5c16af6

Please sign in to comment.