From 25db2c25bc488fcdd10cc5d2fd4938fd78b1cf0e Mon Sep 17 00:00:00 2001 From: Vadim Zeitlin Date: Tue, 23 Oct 2012 23:58:17 +0000 Subject: [PATCH] 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 --- src/common/filename.cpp | 29 +++++++++++++++++++++++++---- tests/filename/filenametest.cpp | 17 +++++++++++++++-- 2 files changed, 40 insertions(+), 6 deletions(-) diff --git a/src/common/filename.cpp b/src/common/filename.cpp index b52e989acd..31251fcdcb 100644 --- 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 4864bf7807..c975cfc363 100644 --- 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__ -- 2.45.2