Disallow executing kate and kwrite as root on Linux
ClosedPublic

Authored by graesslin on Feb 16 2017, 5:09 PM.

Details

Summary

Running GUI applications as root is a huge security risk. Especially
the X server is not secured for that. Non-root applications can easily
interact with a root running application and thus try to exploit simple
bugs in either kate/kwrite itself or in the underlying libraries such
as Qt, XLib or xcb.

In addition kate can be abused to just open the konsole window and any
command can be entered using the XTest extension. This was demonstrated
for dolphin in [1]. The application itself cannot do anything to protect
against it.

On Wayland the situation can be considered worse as the compositor is
running as the normal user and is not protected to handle root windows.
It can be rather trivial to attack the root running application from the
compositor through interfaces such as scripting. This is not in the aim
of the compositors to protect against.

The common use case why users start editors as root is to edit root
owned files. This is a valid use case, but there is no need to run the
application as root. Instead one can use sudoedit to run the application
as user and still be able to edit as root.

This change introduces a check whether the application is started as
root before any interaction with X or Wayland happens, that is prior to
creating the QApplication. If it is detected that we run as root, we
exit and print an information about how to properly edit an application
in kwrite/kate as root. The text is deliberatly not translated to keep
the threat from running as root as low as possible.

The output is:
martin@martin-desktop: ~ $ sudo /opt/kf5/bin/kate
Executing Kate as root is not possible. To edit files as root use:
SUDO_EDITOR=kate sudoedit <file>
martin@martin-desktop: ~ $ sudo /opt/kf5/bin/kwrite
Executing Kate as root is not possible. To edit files as root use:
SUDO_EDITOR=kwrite sudoedit <file>

Test Plan

See output

Diff Detail

Repository
R40 Kate
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
graesslin updated this revision to Diff 11411.Feb 16 2017, 5:09 PM
graesslin retitled this revision from to Disallow executing kate and kwrite as root on Linux.
graesslin updated this object.
graesslin edited the test plan for this revision. (Show Details)
graesslin added a reviewer: Kate.
Restricted Application added a subscriber: kwrite-devel. · View Herald TranscriptFeb 16 2017, 5:09 PM
graesslin updated this revision to Diff 11412.Feb 16 2017, 5:11 PM

Added reference link

I am ok with that. Kate as root is no good idea, KWrite neither.
Other opinions?

brauch added a subscriber: brauch.Feb 16 2017, 5:14 PM

I'm fine with it but there will be complaints. Lots of people open /etc/fstab with kate as root and edit it.

Maybe the sudoedit thing could be done internally when a file name is passed in?

mwolff added a subscriber: mwolff.Feb 16 2017, 6:14 PM

I'm also OK with this. Can we use KAuth to edit files owned by root? Note that they may not even be readable. Maybe we cannot even execute their parent folder...

Anyhow, I'm personally using nano in such cases...

In D4634#86866, @brauch wrote:

I'm fine with it but there will be complaints. Lots of people open /etc/fstab with kate as root and edit it.

Yes exactly that's why I want to change this and educate users about sudoedit.

Maybe the sudoedit thing could be done internally when a file name is passed in?

Unfortunately not. This would require using a QCommandLineParser which requires Q(Core)Application and then you are potentially already owned as that means interaction with X11.

Lets wait if Dominik has a different opinion on that matter, if not, that shall be accepted.

Btw., thanks a lot for caring about security!

Yeah, I figured that was your intention, just saying ;)

Maybe the sudoedit thing could be done internally when a file name is passed in?

Unfortunately not. This would require using a QCommandLineParser which requires Q(Core)Application and then you are potentially already owned as that means interaction with X11.

It doesn't, you can pass in the arguments as string list (http://doc.qt.io/qt-5/qcommandlineparser.html#process) without a QApplication, and even if it would there are other reasonable ways to get a few strings from the argument list, so that's not a hinderance IMO.

dhaumann accepted this revision.Feb 16 2017, 7:56 PM
dhaumann added a reviewer: dhaumann.
dhaumann added a subscriber: dhaumann.

This is a good and reasonable patch, please commit.

This revision is now accepted and ready to land.Feb 16 2017, 7:56 PM
sars added a subscriber: sars.Feb 16 2017, 8:32 PM

At least Ubuntu has a non-zero sudo-timeout and the attack can also be executed on the konsole window after the first sudo command, but I think it is a good idea to educate people about sudoedit. And you have to start somewhere :)

The KWrite patch should have "Executing KWrite as root..." not "Kate"

Parallel of this patch we can provide a service file with 'EDITOR=kate sudoedit %u' as new solution of root actions, what it's used.

Parallel of this patch we can provide a service file with 'EDITOR=kate sudoedit %u' as new solution of root actions, what it's used.

If that works +1, great idea.

apol added a subscriber: apol.EditedFeb 17 2017, 12:32 AM
In D4634#86870, @mwolff wrote:

I'm also OK with this. Can we use KAuth to edit files owned by root? Note that they may not even be readable. Maybe we cannot even execute their parent folder...

There's a discussion on having a GSoC to do this kind of things, I suggest taking a look. (it's centered on dolphin, but given it's basically kio support it's on-topic)

https://marc.info/?l=kfm-devel&m=148390138125694&w=2

This revision was automatically updated to reflect the committed changes.

The KWrite patch should have "Executing KWrite as root..." not "Kate"

fixed

Maybe the sudoedit thing could be done internally when a file name is passed in?

I have been thinking about that one over the weekend as that would be really convenient but don't find a way for it. The problem is: we are already running as root. So when kate gets started we cannot do a sudoedit as that wouldn't give us anything.

So I think that just cannot work. The better approaches would be to integrated the edit as root to service menus, etc.

kkofler added a subscriber: kkofler.Apr 7 2017, 7:23 PM

Sorry, but printing errors to stdout in a GUI application does not make sense. The users will not see them in most cases. Even if you run kdesu (without -t) from a Konsole, it will still not show stdout. There is no alternative to bringing up at least a GUI dialog (other than just stopping patronizing the user that way to begin with). Printing the recommended alternative to somewhere the user will not see is not helpful at all. The user will just see an application that does not run and have no idea why nor how to fix it.

There are also several practical issues that make the recommended alternative of using sudoedit not workable in some use cases, even if you were actually informing the users properly (with a GUI dialog):

  • Kate is by default an MDI "unique application", which completely confuses sudoedit. Only KWrite can be used reliably with sudoedit.
  • sudoedit is of no use if the whole session runs as root, e.g., if you need to do initial system setup, or emergency recovery, and have no working user account.