[SECURITY] Do not configure Krusader path, this is unnecessary
ClosedPublic

Authored by palant on Feb 23 2017, 9:31 AM.

Details

Summary

This is a follow-up to D4725. Not having Krusader path configurable protects against the same scenario: malware running with user's permissions might change the configured Krusader path so that "Start Root Mode Krusader" runs a malicious application with root privileges. While in theory this is less problematic because the user will see the command line they are approving, in practice there are plenty of ways to make it look unsuspicious and most users won't check the command line every time anyway.

There is also a usability aspect here: Krusader might not be installed under PATH, or there might be two Krusader instances installed (particularly for developers). So autoconfiguration won't necessarily do the right thing. In the end, configuring this is unnecessary - there is QCoreApplication::applicationFilePath() which will always point to the correct Krusader instance (namely the one running right now).

I considered adding another special case to KrServices::fullPathName() but Krusader path is being used in exactly one place and using QCoreApplication::applicationFilePath() directly makes the code more obvious.

Diff Detail

Repository
R167 Krusader
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
palant created this revision.Feb 23 2017, 9:31 AM
palant created this object with visibility "Krusader (Project)".
palant created this object with edit policy "Krusader (Project)".
palant updated this revision to Diff 11891.Feb 27 2017, 12:28 PM
palant changed the visibility from "Krusader (Project)" to "Public (No Login Required)".
palant changed the edit policy from "Krusader (Project)" to "All Users".

I updated this patch for changes in D4725 and improved the error message while at it. So now it should be obvious to the user where kdesu is supposed to be.

palant updated this revision to Diff 11892.Feb 27 2017, 12:33 PM

Heh, the important part of this patch got lost in merges. Fixed it again.

abika accepted this revision.Feb 27 2017, 3:44 PM
abika added a subscriber: abika.

Ship it!

But as I already said: I want to remove Krusader root mode anyway.

This revision is now accepted and ready to land.Feb 27 2017, 3:44 PM
martinkostolny accepted this revision.Feb 27 2017, 4:12 PM
This revision was automatically updated to reflect the committed changes.