Bug 99847

Version: kbase3  3.3.92-2005021814-1

CVS commit by faure: 

Check the permissions and ownership of the directory we just created, to see
if it will pass the security checks; otherwise trashing will just fail.
I hate FAT, it breaks all the Unix semantics :)
This fixes "trashing from a USB key fails".
BUG: 99847


  M +53 -25    trashimpl.cpp   1.35


--- kdebase/kioslave/trash/trashimpl.cpp  #1.34:1.35
@@ -200,6 +200,8 @@ bool TrashImpl::createInfo( const QStrin
     // Choose destination trash
     trashId = findTrashDirectory( origPath );
-    if ( trashId < 0 )
+    if ( trashId < 0 ) {
+        kdWarning() << "OUCH - internal error, TrashImpl::createInfo got " << trashId << endl;
         return false; // ### error() needed?
+    }
 
     // Grab original filename
@@ -541,7 +543,8 @@ TrashImpl::TrashedFileInfoList TrashImpl
             if ( fileName == "." || fileName == ".." )
                 continue;
-            Q_ASSERT( fileName.endsWith( ".trashinfo" ) );
-            if ( !fileName.endsWith( ".trashinfo" ) )
+            if ( !fileName.endsWith( ".trashinfo" ) ) {
+                kdWarning() << "Invalid info file found in " << infoPath << " : " << fileName << endl;
                 continue;
+            }
             fileName.truncate( fileName.length() - 10 );
 
@@ -755,6 +758,6 @@ QString TrashImpl::trashForMountPoint( c
     // Minimum permissions required: write+execute for 'others', and sticky bit
     int requiredBits = S_IWOTH | S_IXOTH | S_ISVTX;
-    if ( KDE_lstat( QFile::encodeName( rootTrashDir ), &buff ) == 0
-         && (buff.st_uid == 0) // must be owned by root
+    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
          && (!S_ISLNK(buff.st_mode)) // not a symlink
@@ -770,8 +773,12 @@ QString TrashImpl::trashForMountPoint( c
                 return trashDir;
             }
+                kdDebug() << "Directory " << trashDir << " exists but didn't pass the security checks, can't use it" << endl;
         }
         else if ( createIfNeeded && initTrashDirectory( trashDir_c ) ) {
             return trashDir;
         }
+        } else {
+            kdDebug() << "Root trash dir " << rootTrashDir << " exists but didn't pass the security checks, can't use it" << endl;
+        }
     }
 
@@ -787,4 +794,5 @@ QString TrashImpl::trashForMountPoint( c
             return trashDir;
         }
+        kdDebug() << "Directory " << trashDir << " exists but didn't pass the security checks, can't use it" << endl;
         // Exists, but not useable
         return QString::null;
@@ -801,7 +809,8 @@ int TrashImpl::idForTrashDirectory( cons
     TrashDirMap::ConstIterator it = m_trashDirectories.begin();
     for ( ; it != m_trashDirectories.end() ; ++it ) {
-        if ( it.data() == trashDir )
+        if ( it.data() == trashDir ) {
             return it.key();
     }
+    }
     return -1;
 }
@@ -811,4 +820,13 @@ bool TrashImpl::initTrashDirectory( cons
     if ( ::mkdir( trashDir_c, 0700 ) != 0 )
         return false;
+    // This trash dir will be useable only if the directory is owned by user.
+    // In theory this is the case, but not on e.g. USB keys...
+    uid_t uid = getuid();
+    KDE_struct_stat buff;
+    if ( KDE_lstat( trashDir_c, &buff ) != 0 )
+        return false; // huh?
+    if ( (buff.st_uid == uid) // must be owned by user
+         && (buff.st_mode & S_IRWXU) ) { // rwx for user
+
     QCString info_c = trashDir_c + "/info";
     if ( ::mkdir( info_c, 0700 ) != 0 )
@@ -817,4 +835,14 @@ bool TrashImpl::initTrashDirectory( cons
     if ( ::mkdir( files_c, 0700 ) != 0 )
         return false;
+    } else {
+        kdDebug() << trashDir_c << " just created, by it doesn't have the right permissions, must be a FAT partition. Removing it again." << endl;
+        // Not good, e.g. USB key. Delete again.
+        // I'm paranoid, it would be better to find a solution that allows
+        // to trash directly onto the USB key, but I don't see how that would
+        // pass the security checks. It would also make the USB key appears as
+        // empty when it's in fact full...
+        ::rmdir( trashDir_c );
+        return false;
+    }
     return true;
 }

*** Bug 100669 has been marked as a duplicate of this bug. ***

*** Bug 103385 has been marked as a duplicate of this bug. ***