Don't follow symlinks in wxFileName::Rmdir(wxPATH_RMDIR_RECURSIVE).

Following symlinks, possibly leading outside of the directory being removed,
is at best surprising and at worst dangerous, so don't do it and just mimic
the behaviour of "rm -rf", i.e. remove everything inside this directory,
including the symlinks themselves, but don't follow them.

Closes #14649.

git-svn-id: https://svn.wxwidgets.org/svn/wx/wxWidgets/trunk@72742 c3d73ce0-8a6f-49c7-b76d-6d57e0e08775

diff --git a/src/common/filename.cpp b/src/common/filename.cpp
index b52e989acddd1afc97069b1b659a38194e2b9932..31251fcdcb73a3e4dab2ea91acb12ce1486baca6 100644 (file)
--- a/src/common/filename.cpp
+++ b/src/common/filename.cpp
@@ -1415,6 +1415,21 @@ bool wxFileName::Rmdir(const wxString& dir, int flags)
     if ( flags != 0 )   // wxPATH_RMDIR_FULL or wxPATH_RMDIR_RECURSIVE
 #endif // !__WINDOWS__
     {
+#ifndef __WINDOWS__
+        if ( flags & wxPATH_RMDIR_RECURSIVE )
+        {
+            // When deleting the tree recursively, we are supposed to delete
+            // this directory itself even when it is a symlink -- but without
+            // following it. Do it here as wxRmdir() would simply follow if
+            // called for a symlink.
+            if ( wxFileName::Exists(dir, wxFILE_EXISTS_SYMLINK |
+                                         wxFILE_EXISTS_NO_FOLLOW) )
+            {
+                return wxRemoveFile(dir);
+            }
+        }
+#endif // !__WINDOWS__
+
         wxString path(dir);
         if ( path.Last() != wxFILE_SEP_PATH )
             path += wxFILE_SEP_PATH;
@@ -1426,8 +1441,11 @@ bool wxFileName::Rmdir(const wxString& dir, int flags)
 
         wxString filename;
 
-        // first delete all subdirectories
-        bool cont = d.GetFirst(&filename, "", wxDIR_DIRS | wxDIR_HIDDEN);
+        // First delete all subdirectories: notice that we don't follow
+        // symbolic links, potentially leading outside this directory, to avoid
+        // unpleasant surprises.
+        bool cont = d.GetFirst(&filename, wxString(),
+                               wxDIR_DIRS | wxDIR_HIDDEN | wxDIR_NO_FOLLOW);
         while ( cont )
         {
             wxFileName::Rmdir(path + filename, flags);
@@ -1437,8 +1455,11 @@ bool wxFileName::Rmdir(const wxString& dir, int flags)
 #ifndef __WINDOWS__
         if ( flags & wxPATH_RMDIR_RECURSIVE )
         {
-            // delete all files too
-            cont = d.GetFirst(&filename, "", wxDIR_FILES | wxDIR_HIDDEN);
+            // Delete all files too and, for the same reasons as above, don't
+            // follow symlinks which could refer to the files outside of this
+            // directory and just delete the symlinks themselves.
+            cont = d.GetFirst(&filename, wxString(),
+                              wxDIR_FILES | wxDIR_HIDDEN | wxDIR_NO_FOLLOW);
             while ( cont )
             {
                 ::wxRemoveFile(path + filename);

diff --git a/tests/filename/filenametest.cpp b/tests/filename/filenametest.cpp
index 4864bf78074d89e89404f64e104cdeef23b5cc79..c975cfc363f72aa59fadca628ad1ac09e646686b 100644 (file)
--- a/tests/filename/filenametest.cpp
+++ b/tests/filename/filenametest.cpp
@@ -784,11 +784,12 @@ void FileNameTestCase::TestSymlinks()
     wxFileName targetfn(wxFileName::CreateTempFileName(tempdir));
     CPPUNIT_ASSERT(targetfn.FileExists());
 
-    // Create a symlink to that file, and another to the home dir
+    // Create a symlink to that file
     wxFileName linktofile(tempdir, "linktofile");
     CPPUNIT_ASSERT_EQUAL(0, symlink(targetfn.GetFullPath().c_str(),
                                         linktofile.GetFullPath().c_str()));
 
+    // ... and another to the temporary directory
     const wxString linktodirName(tempdir + "/linktodir");
     wxFileName linktodir(wxFileName::DirName(linktodirName));
     CPPUNIT_ASSERT_EQUAL(0, symlink(tmpfn.GetFullPath().c_str(),
@@ -925,8 +926,20 @@ void FileNameTestCase::TestSymlinks()
     CPPUNIT_ASSERT(!wxFileName(tempdir, "linktofile").Exists());
     CPPUNIT_ASSERT(linktofile.Exists());
 
-    // Clean-up, and also tests removal of a dir containing a symlink-to-dir
+    // This is also a convenient place to test Rmdir() as we have things to
+    // remove.
+
+    // First, check that removing a symlink to a directory fails.
+    CPPUNIT_ASSERT( !wxFileName::Rmdir(linktodirName) );
+
+    // And recursively removing it only removes the symlink itself, not the
+    // directory.
+    CPPUNIT_ASSERT( wxFileName::Rmdir(linktodirName, wxPATH_RMDIR_RECURSIVE) );
+    CPPUNIT_ASSERT( tmpfn.Exists() );
+
+    // Finally removing the directory itself does remove everything.
     CPPUNIT_ASSERT(tempdirfn.Rmdir(wxPATH_RMDIR_RECURSIVE));
+    CPPUNIT_ASSERT( !tempdirfn.Exists() );
 }
 
 #endif // __UNIX__