Summary: | Directory permission checks for .Trash directory are to restrictive | ||
---|---|---|---|
Product: | [Frameworks and Libraries] kio | Reporter: | Stefan Brüns <stefan.bruens> |
Component: | trash | Assignee: | David Faure <faure> |
Status: | RESOLVED FIXED | ||
Severity: | normal | ||
Priority: | NOR | ||
Version: | unspecified | ||
Target Milestone: | --- | ||
Platform: | unspecified | ||
OS: | Linux | ||
Latest Commit: | Version Fixed In: | ||
Attachments: | trashdir.diff |
Description
Stefan Brüns
2007-02-24 17:14:52 UTC
You are right. I think I special-cased "the directory has to be writable by the user", but instead of assuming a certain uid + permissions I should just use access()... What do you think of the attached patch? Created an attachment (id=19832) trashdir.diff Any feedback on the patch? Seems to work, actually, I'll commit. SVN commit 660968 by dfaure: [Bug 142148] Directory permission checks for .Trash directory are to restrictive It's actually fine if root-owned .Trash isn't readable/writable by users, users only need to be able to access their own subdir. BUG: 142148 M +5 -5 trashimpl.cpp --- branches/KDE/3.5/kdebase/kioslave/trash/trashimpl.cpp #660967:660968 @@ -786,16 +786,16 @@ // (1) Administrator-created $topdir/.Trash directory const QString rootTrashDir = topdir + "/.Trash"; + const QCString rootTrashDir_c = QFile::encodeName( rootTrashDir ); // Can't use QFileInfo here since we need to test for the sticky bit uid_t uid = getuid(); KDE_struct_stat buff; - // Minimum permissions required: write+execute for 'others', and sticky bit - uint requiredBits = S_IWOTH | S_IXOTH | S_ISVTX; - if ( KDE_lstat( QFile::encodeName( rootTrashDir ), &buff ) == 0 ) { - if ( (buff.st_uid == 0) // must be owned by root - && (S_ISDIR(buff.st_mode)) // must be a dir + const uint requiredBits = S_ISVTX; // Sticky bit required + if ( KDE_lstat( rootTrashDir_c, &buff ) == 0 ) { + if ( (S_ISDIR(buff.st_mode)) // must be a dir && (!S_ISLNK(buff.st_mode)) // not a symlink && ((buff.st_mode & requiredBits) == requiredBits) + && (::access(rootTrashDir_c, W_OK)) ) { const QString trashDir = rootTrashDir + "/" + QString::number( uid ); const QCString trashDir_c = QFile::encodeName( trashDir ); SVN commit 916987 by dfaure: Fix wrong fix for 142148, I missed the ==0 so the logic was reversed, oops. CCBUG: 142148 CCBUG: 178479 This fix will be in 4.2.1, probably not in 4.2.0. M +4 -2 trashimpl.cpp WebSVN link: http://websvn.kde.org/?view=rev&revision=916987 |