From 66c4a258c4ea7e55a171186bbe4e0f2c875bbc24 Mon Sep 17 00:00:00 2001 From: Josh Cooper Date: Thu, 27 Jun 2024 17:51:06 -0700 Subject: [PATCH] (PUP-9997) Remove call to Dir.chdir when managing symlinks Symlinks can refer to their target using a relative or absolute path. If a relative path is used, then it's assumed to be relative to the symlink's parent directory. For example, /tmp/link refers to /tmp/target using a relative path: $ ls -l /tmp/link lrwxrwxrwx 1 josh 6 Jun 27 23:26 /tmp/link -> target In commit 02f91fcd55, symlinks were merged into the file (aka pfile) type. The `ensure` property was modified to validate whether the target existed or was a directory[1] In order for relative symlinks to be validated, it was necessary to call `Dir.chdir`. Later the call to `Dir.chdir` was moved to `target.rb`[2], and then the `FileTest.exists?(target)` check was deleted[3]. This commit removes the call to `Dir.chdir`. This means we no longer change our cwd prior to dropping privileges and creating the symlink. This should be ok because none of the code within the Dir.chdir block is sensitive to cwd. Also when we reach this block of code, we're committed to creating the symlink given an absolute `@resource[:path]`, so we don't need to be concerned about TOCTOU race conditions. [1] https://github.com/puppetlabs/puppet/blob/02f91fcd55aefe63a11a29c5608c0738867b8130/lib/puppet/type/pfile/ensure.rb#L76-L86 [2] https://github.com/puppetlabs/puppet/blob/2cd67ad843409a49747748a1278e5ef788dd97bd/lib/puppet/type/pfile/target.rb#L41-L45 [3] https://github.com/puppetlabs/puppet/commit/662fcafdb8020b34b060d70d54fca996ece36ab8#diff-03f0464c0f42c4b979f6150ec2f7e8043ec964fe4d401cd0cd57a82f43a856ba --- lib/puppet/type/file/target.rb | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/lib/puppet/type/file/target.rb b/lib/puppet/type/file/target.rb index 973e5ddf9fa..b7cd6b62a9e 100644 --- a/lib/puppet/type/file/target.rb +++ b/lib/puppet/type/file/target.rb @@ -44,22 +44,20 @@ def mklink raise Puppet::Error, "Could not remove existing file" if Puppet::FileSystem.exist?(@resource[:path]) - Dir.chdir(File.dirname(@resource[:path])) do - Puppet::Util::SUIDManager.asuser(@resource.asuser) do - mode = @resource.should(:mode) - if mode - Puppet::Util.withumask(0o00) do - Puppet::FileSystem.symlink(target, @resource[:path]) - end - else + Puppet::Util::SUIDManager.asuser(@resource.asuser) do + mode = @resource.should(:mode) + if mode + Puppet::Util.withumask(0o00) do Puppet::FileSystem.symlink(target, @resource[:path]) end + else + Puppet::FileSystem.symlink(target, @resource[:path]) end + end - @resource.send(:property_fix) + @resource.send(:property_fix) - :link_created - end + :link_created end def insync?(currentvalue)