Hiding filename/extention for .desktop files with execute permission.

Mathias Svanbäck submitted a patch:

Just to indicate the filetype to the user of what kind of extention it actually is.

Patch 6981, "Could maybe append the .desktop extention to the display_name while in the filebrowser.":
file_13329.txt

Mathias Svanbäck said:

Hosting the suggested patch on my github

https://github.com/MatteO-Matic/thunar/tree/Matte_ShowDesktopExtention_20170201_070840

Yves-Alexis Perez said:

The same kind of issue has been assigned CVE-2017-14604 in Nautilus. See also

https://micahflee.com/2017/04/breaking-the-security-model-of-subgraph-os/ and https://bugzilla.gnome.org/show_bug.cgi?id=777991 https://github.com/GNOME/nautilus/commit/1630f53481f445ada0a455e9979236d31a8d3bb0

The executable bit protection can be somehow bypassed by for example shipping a tarball which would be extracted by an user. For Nautilus it's even worse because apparently if the .desktop file is called foo.desktop.pdf it'll be displayed as a PDF icon but handled as a .desktop file.

Nautilus fixed it by storing the “executable” / “trusted” information in a metadata, which is apparently a gio/gvfs stuff, stored on the filesystem in XDG_DATA_DIR/gvfs-metadata (usually .local/share/gvfs-metadata), which is supposingly not reachable when extracting a tarball (unless there's a directory traversal vulnerability in the extraction process).

I'm not sure if something like that applies to Thunar, but it'd be nice to have additional hardening.

Alexander Schwinn @alexxcons submitted a patch:

As a first step, I would prefer to just display the real filename, instead of "Startername".desktop, which possibly will add more confusion. (That's what Nautilus and as well Dolphin do ) Could be pushed soon, so that's available directly in xfce 4.14 Would that be fine for you ? (Please test the attached patch if you have time)

Regarding the second step, internally saving the executable bit: Since gvfs is optional for thunar, I dont think we should follow the nautilus aproach of storing the exec bit in gvfs.

Instead we possibly could store a whitelist of all .desktop files allowed to be executed in xfconf (full path) instead of relying on the executable bit. Both, thunar and xfdesktop should use the list to check execution permission. A drawback: Renaming a .desktop file would require a second confirmation .. though I think this would be tolerable. Any arguments against the proposal ?

Patch 8471, "Show real filename for .desktop files":
0001-Hiding-filename-extention-for-.desktop-files-with-ex.patch

added 5. Mime Types and .desktop Files label

mentioned in merge request !41 (merged)

That's how it is done for nautilus:

https://bugzilla.gnome.org/show_bug.cgi?id=777991#c4

https://github.com/GNOME/nautilus/commit/1630f53481f445ada0a455e9979236d31a8d3bb0

Note that it is important to clean the trusted flag when a .desktop file is removed, moved or renamed in order to make sure that a possible leftover gvfs trusted flag is not abused by another malicious launcher. (See comments in https://bugzilla.gnome.org/show_bug.cgi?id=777991) Not sure if nautilus does that.

I just uploaded a proof-of-concept of using trusted flag GVFs-metadata. A new metadata named metadata::thunar-exe-digest is used to store SHA-256 of that file if user marked it as an executable. Although the value of metadata::thunar-exe-digest can be set as skip-digest, safety flag without proper digest will not be copied or kept when file is moved or copied.

This branch will never be merged into master since it is a combinaton of soon-to-be-implemented-on-xfce4util and an ad-hoc patch to test new methods.

I'll post a merge request to xfce4util after its API is polished enough.

Thanks alot ! Excellent idea to use the a hash value to make sure the .desktop file was not exchanged !

I'll test & have a rough look at the code tomorrow.

Probably better to name the metaitem metadata::xfce-exe-digest, since as well xfdesktop (and others?) should/will make use of it. I suppose the word hash is more common in informatics. How about metadata::xfce-exe-hash ?

Regarding skip-digest, I did not understand the related paragraph. It's not yet clear to me what would be a use-case for it. To copy/move a .desktop file will anyhow require a new meta-item to be created for that file, using the same SHA-256 like the old file. Or am I wrong ?

Probably better to name the metaitem metadata::xfce-exe-digest

That is a silly mistake! Even though I was writing for libxfce4util I still use thunar namspace apparently.

How about metadata::xfce-exe-hash ?

That would be straightforward.

To copy/move a .desktop file will anyhow require a new meta-item to be created for that file

G_FILE_ATTRIBUTE_INFO_COPY_WITH_FILE | G_FILE_ATTRIBUTE_INFO_COPY_WHEN_MOVED will do the job automatically.

Regarding skip-digest, I did not understand the related paragraph. It's not yet clear to me what would be a use-case for it.

I thought it would be useful for some cases but it would make hashing useless. It would be better to force hash check everytime.

I thought it would be useful for some cases but it would make hashing useless. It would be better to force hash check everytime.

Ok so lets just enforce hashing for now. If we later on feel that "skip-hash" will be needed for some use-case, we can add the functionality.

A small "nice to have": When gvfs is disabled, currently an infobox is shown in the "advanced" tab in the settings. It would be good to as well inform about the security implications related to .desktop files in that infobox.

mentioned in issue #56 (closed)

changed milestone to %Xfce 4.18

mentioned in issue #50 (closed)

Ok finally had time to do some testing:

Directly after first start, the execute flag of all .desktop files in thunar is set to false. No icon is displayed for these files and activating them results in getting them opened via mousepad. That is very inconvenient, since the user would need to set the execute flag for them (assuming the user knows about it) I would propose the following:
- 1.) As before, activation should result in an dialog to ask the user if the .desktop file can be trusted
- 2.) I think we should have a way to migrate "only execute flag set" to "SHA-256 generated". So users dont need to trust each single file again. E.g. if no SHA-256 was generated for a whole folder of .desktop files, we could ask, if all .desktop files in that folder should be migrated/trusted instead of the old dialog. That could as well be useful if thunar is installed into some non-xfce system. What do you think on this aspect ? Maybe there is a better way ? In any way, not required to do all that at once, could be done in a separate MR.
- 3.) (optional) It would be nice to display icons, even though the .desktop file is not yet trusted (like xfdesktop does) ... that would be a new feature, so possibly better to have it as separate issue.
There is a strange effect, when opening the desktop folder via thunar, and than launching a .desktop file via xfdesktop: It looks like the execute flag got removed. If I hit "mark as executable" the application will start, but on the next try, the flag is lost again. So it seems like thunar writes the flag "on file change".

Added some comments directly on the commits of the PoC:

https://gitlab.xfce.org/MShrimp4/thunar/-/commit/4b2154ac631bca7c1bb6ba186a9ea9054842b2de https://gitlab.xfce.org/MShrimp4/thunar/-/commit/e07879a090aa2f50ec91331d2b8aefdbc779af2b

(Only commented on general things, I did not look into memory management, style, etc. )

@alexxcons This is a second attempt. This time, I separated permission bit and safety flag. gvfs-safety-flag-test

By the way, do you think that new gio extensions are good enough to be merged into libxfce4util?

By the way, do you think that new gio extensions are good enough to be merged into libxfce4util?

Generally yes. Though some details will need to be discussed in the libxfce4util MR. Please feel free to open a MR for it whenever you think it is ready !

Suppose you want to modify xfce_g_file_is_executable/xfce_g_file_set_executable first in order to differ between exe-bit and gvfs-meta value.

WRT to the dialog checkbox: An extra-checkbox like that would be fine for me. Though I wonder why you dont have the additional tab "launcher" on your properties window :

If it is available by default (seems not to be the case), that's where I would put that extra-checkbox, since it is anyhow only relevant for launchers. The text should not mention thunar, because as well xfdesktop should use the flag. Maybe "Allow this launcher to be executed" ?

WRT to the dialog checkbox: An extra-checkbox like that would be fine for me. Though I wonder why you dont have the additional tab "launcher" on your properties window :

That page is an extension provided by thunar-apr. I suppose that is not enabled when it's being tested? I used the method on the wiki page.

Since it also works on shell scripts and executables, it would be natural to put where it's on right now though.

That page is an extension provided by thunar-apr. I suppose that is not enabled when it's being tested? I used the method on the wiki page.

Oh it's thunar-apr, thanks for investigation ! :) It is bundled with thunar, so we theoretically could make use of it. Though yes, if you plan to use the flag as well for scripts and executables, the "launcher" tab is the wrong place.

Since it also works on shell scripts and executables, it would be natural to put where it's on right now though.

Ok for me to use it as well for shell scripts and executables. I thought you wanted to do launchers-only, because of the screenshot you provided. So we would need a different naming. How about this:

Execution: [x] This file is executable
           [x] This file is trusted

A tooltip on each checkbox could give further information. E.g. something like:

"If this flag is set, the file can be executed via command line. If you want to execute it as well via xfce applications, you additionally need to set the 'trusted' flag"

"Set this flag if you trust the according file to do no harm to your system. When set, you can execute the program via xfce application. The flag is stored as gvfs-metadata, not with the file itself. This is done because files shipped via an archive (specially launchers) can already have the executable flag set and might be used to trick the user to execute malicious code."

I think setting "trusted" should automatically as well set the "executable" flag, but unsetting it should not unset "executable".

Execution: [x] This file is executable
           [x] This file is trusted

It would be much better that way. I'll change the UI accordingly.

I think setting "trusted" should automatically as well set the "executable" flag, but unsetting it should not unset "executable".

That was unintended, so I'll fix it later.

mentioned in merge request libxfce4util!16 (merged)

My two cents: I think this trusted flag makes sense for desktop files as they can disguise as anything, but scripts or executables cannot. IMO security is indeed important but convenience shouldn't be blindly given up.

I suggest starting with desktops files, later (in another issue) we can consider expanding this flag to other file types.

but convenience shouldn't be blindly given up.

Ok, possibly I was a bit light-minded here. Yes, makes sense to only introduce extra-hurdles if there is a real risk. And the risk to be tricked, only seems to be present for ".desktop" files. (Correct me if I should be wrong)

Sorry @MShrimp4 for being shaky here. So lets not touch the "permissions" tab, but add the new checkbox into the "launcher" tab. I think the naming we agreed on still is good if we just swap 'file' for 'launcher'.

Execution: [x] This launcher is trusted

If this flag is going to be applied only to .desktop files then I am by no means against putting on "launcher" tab.

Well, actually, I do have a concern now:

Even though thunar-apr is a bundled plugin, it is a plugin after all. Is it fine to have a flag globally used throughout XFCE and leave its interface optional?
Since enabling "safety flag" should also enable execution flag, wouldn't it be better to have these two on the same page and show the user visually that these two are correlated?

Thank you for raising the concerns ! I hope I can clear them:

Regarding 1:

Per default, thunar-apr is build and installed together with thunar. But yes, it seems to be possible to disable it by passing --disable-apr-plugin (Though I dont think anybody does so)

Though if a non-trusted .desktop file is activated, anyhow some dialog should popup in order to ask the user if the file should be trusted. So thunar-apr should actually not be needed to use the feature.

xfdesktop and as well the panel use some other GUI in order to modify launchers. xfdesktop has a menu-entry "edit launcher" for it. For consistency we maybe should as well add that context-menu-entry to thunar and call the same GUI. (independent from this issue)

Regarding 2:

As long as we keep the feature .desktop-files-only, I think it is better to have the "trusted" checkbox only on the launcher tab. How about displaying "is executable" additionally on the launcher tab to make the correlation more clear ?:

Execution: [x] This launcher is executable
           [x] This launcher is trusted

Though if a non-trusted .desktop file is activated, anyhow some dialog should popup in order to ask the user if the file should be trusted. So thunar-apr should actually not be needed to use the feature.

Yes, it is a builtin feature. thunar_dialogs_show_insecure_program() returns gboolean based on user response.

How about displaying "is executable" additionally on the launcher tab to make the correlation more clear ?:

I think that's a better idea.

Great, so I think we all agree now

I opened an MR for this since gio extension parts are now separated. !121 (merged)

mentioned in commit libxfce4util@c881265b

added 8. GSoC 2021 label

closed with commit 888ee366

Fixed by !121 (merged) thanks to @MShrimp4, to be released in thunar 4.17.4

mentioned in issue #959 (closed)

mentioned in issue panel-plugins/xfce4-whiskermenu-plugin#91 (closed)

mentioned in issue #1021 (closed)

Hiding filename/extention for .desktop files with execute permission.

Submitted by Mathias Svanbäck

Description

Designs

Child items ...

Activity