First work on .desktop file brouhaha [PATCH]
Michael Pyne
mpyne at purinchu.net
Thu Feb 19 04:27:59 GMT 2009
I've done a bit of working on making the .desktop files less likely to cause
grief for unwitting KDE users (as mentioned on LWN.net and Slashdot).
The following patch (also attached) is my first attempt at implementing a
modicum of security while also allowing most .desktop files to work as is.
I'll go ahead and include it inline here and then afterwards include my
discussion:
Index: kinit/klauncher.cpp
===================================================================
--- kinit/klauncher.cpp (revision 927231)
+++ kinit/klauncher.cpp (working copy)
@@ -774,16 +774,50 @@
return start_service(service, urls, envs, startup_id.toLocal8Bit(), blind,
false, msg);
}
+// helper function for start_service()
+static bool
+isStandardService(const char *type, const QString &servicePath)
+{
+ QStringList resourceDirs = KGlobal::dirs()->resourceDirs(type);
+ foreach(QString resourceDir, resourceDirs) {
+ if(resourceDir == servicePath)
+ return true;
+ }
+
+ return false;
+}
+
bool
KLauncher::start_service(KService::Ptr service, const QStringList &_urls,
const QStringList &envs, const QByteArray
&startup_id,
bool blind, bool autoStart, const QDBusMessage &msg)
{
QStringList urls = _urls;
- if (!service->isValid())
+ QString servicePath = service->entryPath();
+ bool runPermitted = !QDir::isAbsolutePath(servicePath) || // relative
paths will look in standard dirs
+ QFileInfo(servicePath).isExecutable() || //
executable files are OK
+ !QFileInfo(servicePath).isWritable(); //
presumably so are files the user can't create
+
+ if (!runPermitted) { // Path was absolute, double-check it's in standard
directories
+ QFileInfo serviceInfo(servicePath);
+ QString canonicalService = serviceInfo.canonicalPath(); // Work if
.desktop file is found from a symlink
+
+ const char *const serviceTypes[] = {"apps", "services", "xdgdata-
apps"};
+ // sizeof stuff adapts to changes in array length
+ for(size_t i = 0; i < sizeof(serviceTypes) / sizeof(serviceTypes[0]);
++i) {
+ if(::isStandardService(serviceTypes[i], canonicalService)) {
+ runPermitted = true;
+ break;
+ }
+ }
+ }
+
+ if (!service->isValid() || !runPermitted)
{
requestResult.result = ENOEXEC;
requestResult.error = i18n("Service '%1' is malformatted.", service-
>entryPath());
+ if (service->isValid())
+ requestResult.error = i18n("Service '%1' must be executable to
run.", service->entryPath());
cancel_service_startup_info( NULL, startup_id, envs ); // cancel it if
any
return false;
}
Discussion: The patch works by checking the .desktop file in
KLauncher::start_service() (which seems to be called indirectly by basically
any path in kdelibs to run a service). The check consists of:
* Is the path to the file relative? If so permit it for now as it will be
searched for later (this is my assumption at least, I have not gone through
the code paths to verify).
* If not, is the file executable? (I'm not sure if this needs Q_OS_WIN masking
or anything however).
* If still not, could the user create the file? If not allow it.
In the event of an absolute path further checks are conducted in order to
verify that the path to the desktop file is one of the resource dirs used by
KStandardDirs for the following types: "apps", "services", "xdgdata-apps".
(Does this cover all the types that need to be searched?). The path to the
service is canonicalized so that a symlink to an installed service should
still work (I didn't actually test. ;)
Assuming the check does not pass an error message is returned that the service
must be executable (but the same ENOEXEC error code is returned as I feel
that's accurate, do we need a different one?)
I have not implemented auto-executable-bit-ifying .desktop files (I think that
would go into libkonq?) Also, is there other code paths I need to look for in
kdelibs that end up directly executing .desktop files (everything I've used so
far has been blocked appropriately in my testing, kioclient, krunner, kickoff,
and dolphin).
Standing by for comments (although it will be some hours before I'm online
again most likely).
Regards,
- Michael Pyne
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090218/9942a310/attachment.htm>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: fix-desktop-file-security.patch
Type: text/x-patch
Size: 2342 bytes
Desc: not available
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090218/9942a310/attachment.bin>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 197 bytes
Desc: This is a digitally signed message part.
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20090218/9942a310/attachment.sig>
More information about the kde-core-devel
mailing list