Hey, as you know, there is a “security” issue in the way Thunar (and others) handles .desktop files. In fact, the security issue is that there is no security planned in the spec. The problem is that it's easy for people to ship nasty .desktop files to users (via mail or browser), and once they are saved in the Desktop, they can be easily run and do nasty stuff. One example is attached. Thunar currently has some way to detect malwares, but it's not enough for the attached one, for example. There was a lot of noise about this recently, and I'm not that sure it's that grave, but it's a security issue anyway. The solution proposed is to prevent all .desktop to be executed, except if: - they are installed in standard prefix / owned by root - are locally installed, and have the +x bit Or there is the idea to use a shebang for desktop files, using an xdg-launch which would check all that. The problem is that, currently, there is no spec for that so in the end all DE could do that a different way, which wouldn't be really convenient. GNOME people seem to do this that way: http://mail.gnome.org/archives/desktop-devel-list/2009-February/msg00132.html If you need more info, help etc, please ask :) Cheers,
Created attachment 2200 Malware desktop file
Created attachment 2456 Rewrite of thunar_file_is_desktop_file() What about a desktop file check like the one attached? Checks for the following things (hopefully, because I've not compiled it). * Must have a real .desktop file extension * Content type must be application/x-desktop * Need the +x flag or is installed in a system data dir.
Created attachment 2457 Same as other patch, but also check user's data dir.
Yeah it looks good. Will this benefit to xfdesktop too?
(In reply to comment #3) > Created an attachment (id=2457) [details] > Same as other patch, but also check user's data dir. I'm not sure doing this in thunar_is_desktop_file() is the correct thing to do. This function is also used to display a desktop file editor tab in file properties dialogs and just because a desktop file should not be executed doesn't mean users shouldn't be able to edit it. thunar_file_is_executable() might be wrong as well. We don't want to open desktop files in text editors, no matter if they are executable or not. Instead, we want to provide execute even for desktop files that should not be executed. When the user then tries to execute such a file, we want to present an error explaining why this won't work. So the correct place for these changes most likely is thunar_file_execute(). I think that, along with these changes, we also want to add some kind of "[ ] Mark as trusted" checkbox in the desktop file editor tab (as suggested by Vincent Untz in the GNOME ML thread) for desktop files that are writable.
Created attachment 2459 svn diff dump Played with this but it will give some problems too, for example existing desktop files on the desktop. Until there is a complete solution for this exploit, we leave it alone.
Created attachment 2460 IRC discussion about this.
From YAP's link it appears that part of this is already implemented in GIO, though IMHO the less useful part: content sniffing is disabled for .desktop files, so files that are .desktop files but have a weird extension won't get marked executable. As for something stronger, I'll summarize my position (so you don't have to read through the IRC discussion. Any solution we implement must: 1. Actually increase security, and not just give the appearance of it. 2. Have a decent, non-intrusive migration/upgrade path. If our solution means the old .desktop files on ~/Desktop (or wherever) that used to work stop working without user interaction, that's bad. The best I can come up with so far is this: 1. Allow .desktop files in root-owned directories to be executable as-is. 2. Any .desktop files not owned by the current user are not executable. 3. Any .desktop files owned by the current user, do not have the execute (+x) permission bit set, and were created/modified after the first time Thunar was run with this feature enabled, are not executable. 4. Any .desktop files owned by the current user with the execute permission bit set are executable. For cases #2 and #3, we'd present a dialog box to the user that says something like this: The file <filename> that launches the application <app name> is untrusted and may be an attempt to get you to run malicious software. If you trust this file, you may continue, but if not, it is best not to run it. [Do Not Launch] [Launch <app name>] [Launch <app name> and Mark Trusted] Instead of (or in addition to) presenting the application's name, we might want to present the Exec= line as well. Or perhaps have a "Details" expander in the dialog that presents more info. The "Mark Trusted" button could instead be a checkbox for "Remember this decision for later" (though the intention is that it would only remember the "trust" action, and still present the dialog again otherwise). Trusting the file amounts to setting +x on it. Of course, we can't do +x on files not owned by the user, so we may hide that button in the dialog if that's the case. I still don't really like even this proposal, but that's the best I can come up with.
As an aside, one issue that isn't covered here is if some malicious application modifies an already-trusted file (one that already has +x) to do something bad. Of course, if a malicious application is already running, it could set +x on whatever files it wishes anyway. I guess that's my issue with trying to fix this anyway. Once malware is running, any measures we take here are irrelevant. +x can be added to any file by the app, any .desktop file can be modified, and any list of trusted .desktop files we keep somewhere can be modified. Of course, preventing malware from running in the first place is a laudable goal of fixing this issue, but it's not the entire picture. While this might help prevent a user from unwittingly running something nasty, there are likely other ways an attacker could run code on the user's machine without user intervention (an exploitable vulnerability in the web browser, for example).
(In reply to comment #9) > While this might help prevent a user from unwittingly > running something nasty, there are likely other ways an attacker could run code > on the user's machine without user intervention (an exploitable vulnerability > in the web browser, for example). Yes but that doesn't mean nothing should be done to prevent other ways to compromise the machine.
Hey guys, I'm quite impressed that this bug doesn't really bother you. In fact, it is not a security hole, it is a giant crater. Resolution from nautilus is simple: Check if *.desktop has +x privileges: a) yes: launch it! b) no: throw a popup with notification, that we know nothing about this file and let user decide if he is sure it is safe. Dolphin approach is similar with one noticeable difference: it shows in popup code from 'Exec='.
Hey, any news on this. It'd be really nice if this could be fixed for 4.8. Proposed solutions looked good (even if the spec still doesn't have anything on this), so...
I guess the proposed nautilus/dolphin solution makes sense, although it is really quick&dirty and doesn't really protect the user. I don't know about string freeze, but otherwise the patch is easy.
So this was not fixed for 4.8, it'd be nice to have it for 4.10 :)
Fixed in 1ec8ff8