Bug 435696 - Validate relative paths in desktop files for X-KDE-DBUS-Restricted-Interfaces
Summary: Validate relative paths in desktop files for X-KDE-DBUS-Restricted-Interfaces
Status: RESOLVED INTENTIONAL
Alias: None
Product: kwin
Classification: Plasma
Component: platform-wayland-nested (show other bugs)
Version: unspecified
Platform: Arch Linux Linux
: NOR normal
Target Milestone: ---
Assignee: KWin default assignee
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2021-04-13 16:10 UTC by Gena
Modified: 2021-04-14 11:43 UTC (History)
3 users (show)

See Also:
Latest Commit:
Version Fixed In:
Sentry Crash Report:


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description Gena 2021-04-13 16:10:03 UTC
SUMMARY

At the moment, to take a screenshot on Walyand, the `X-KDE-DBUS-Restricted-Interfaces = org.kde.kwin.Screenshot` parameter must be specified in the .desktop file. But this only works for absolute paths specified in `Exec=`. Meven suggested to handle relative paths in this comment: https://phabricator.kde.org/D29407#inline-171509, it has not been resolved.

ADDITIONAL INFORMATION

My application (https://github.com/crow-translate/crow-translate) uses text recognition from the screen and in order to be able to capture the screen on Walyand I need to hardcoded the full path in the .desktop file. This creates some inconvenience, for example, for creating an AppImage.

The standard allows the use of relative paths: https://specifications.freedesktop.org/desktop-entry-spec/desktop-entry-spec-latest.html#exec-variables
Comment 1 David Redondo 2021-04-14 07:27:46 UTC
I think absolute paths are intentional. If you want to ship your app in some packaged format you might want to look into the cross-desktop screenshot portal.
Comment 2 Gena 2021-04-14 08:04:56 UTC
(In reply to David Redondo from comment #1)
> I think absolute paths are intentional. If you want to ship your app in some
> packaged format you might want to look into the cross-desktop screenshot
> portal.

On KDE Plasma, the portal first shows a pop-up window with a preview in which the user has to click "Save", after which I can display a selection of the rectangular recognition area. I already support this, but it is not very obvious to the user, so I would like to use the native API on KDE.
If security is so important, can we check the resulting path from $PATH and allow taking a screenshot if the application folder is owned by root?
Comment 3 David Edmundson 2021-04-14 09:44:40 UTC
>I think absolute paths are intentional

It is. Otherwise the security aspect is just pointless, anyone can call their app spectacle and take as many screenshots as they want.

Our current implementation does need revisiting, there's lots of ways round it for anything not in a sandbox (which I won't list publicly), and we'll consider this use case when that happens.

But for now this is currently working as-intended and won't be changed.
Comment 4 Gena 2021-04-14 09:57:43 UTC
(In reply to David Edmundson from comment #3)

> It is. Otherwise the security aspect is just pointless, anyone can call
> their app spectacle and take as many screenshots as they want.

I agree, this is why I suggested to just check the folder owner if the application executable was taken from PATH. What do you think about this?
Comment 5 David Edmundson 2021-04-14 10:35:33 UTC
Then you break everyone's dev setups, and you have snaps and flatpaks in your path, which breaks real sandboxes which is a major no-no.

It also goes against what this code is /trying/ to do with per-app whitelists, which is of debatable significance.
Comment 6 Gena 2021-04-14 11:43:55 UTC
(In reply to David Edmundson from comment #5)
> Then you break everyone's dev setups, and you have snaps and flatpaks in
> your path, which breaks real sandboxes which is a major no-no.

You probably misunderstood me. I suggested adding support for executables from PATH, but additionally validating paths that obtained from PATH. For example, check if the received application location from PATH is `/usr/bin/share` (or any other folder owned by root). This approach shouldn't break anything, and full paths will work as before (e.g. without additonal check).