From 5c16af6c93926e679ed0a3175ef269147d8b599a Mon Sep 17 00:00:00 2001 From: "Haelwenn (lanodan) Monnier" Date: Thu, 30 Jan 2025 00:00:29 +0100 Subject: [PATCH] File.mkdir_p: Fix TOCTOU issue & handle errors from parent mkdir 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 --- lib/elixir/lib/file.ex | 29 ++++++++++++++-------------- lib/elixir/test/elixir/file_test.exs | 22 +++++++++++++++++++++ 2 files changed, 36 insertions(+), 15 deletions(-) diff --git a/lib/elixir/lib/file.ex b/lib/elixir/lib/file.ex index 0c3e79931fb..a85d105c832 100644 --- a/lib/elixir/lib/file.ex +++ b/lib/elixir/lib/file.ex @@ -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 diff --git a/lib/elixir/test/elixir/file_test.exs b/lib/elixir/test/elixir/file_test.exs index 47301ce1a7d..c053a1be76c 100644 --- a/lib/elixir/test/elixir/file_test.exs +++ b/lib/elixir/test/elixir/file_test.exs @@ -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