Review Request 108711: kcmdatetimehelper: Hardcode PATH because $PATH is empty here.

[Kevin Kofler]

> On Feb. 4, 2013, 3:53 p.m., Konstantinos Smanis wrote:
> > We can do better than hardcoding a reasonable default. We can launch a login shell (1) and 'echo $PATH' the user's PATH. This has many advantages:
> > 
> > 1. We don't miss paths (e.g. /usr/local/bin, /usr/local/sbin etc.).
> > 2. We honor the precedence of paths as set in $PATH by the user.
> > 3. We only use the user's PATH (DBus activation works for non-root users too).
> > 
> > I am currently working on this approach for kcm-grub2 which also misbehaves when $PATH is not set. If you are interested, you may restrain from committing until I post a link to the commit.
> > 
> > (1) A login shell is needed to properly source /etc/profile, ~/.profile and/or other shell-specific login scripts (such as ~/.bash_profile for Bash).
> 
> Konstantinos Smanis wrote:
>     Here you are: http://commits.kde.org/kcm-grub2/7c5beb979fdf9dd14abfffb0e24d4f69b11ca985
> 
> Thomas Lübking wrote:
>     Question (for the particular case and in general):
>     This is about a suid root:dbus call (thus env clearing for dbus system activation) correct?
>     Ie. the called application is executed with root privs?
>     
>     In this case there should afaics rather not be _any_ PATH resolution at all and checking the usual suspects is about the last acceptable process.
>     
>     Otherwise one could place a random binary "zic" or "hwclock" into the $PATH and gain a root shell (or are there further protections against this?)

* This code (and any KAuth helper, actually) always runs as root.
* We're looking for 2 core system executables, the chances they are in /usr/local are near zero, and I'd also share Thomas Lübking's security concerns. (D-Bus clears the path for a reason.)
* Spawning a login shell looks like a really overengineered and ugly solution, considering the above.
* kcm-grub2 is the last piece of software I'd model a KAuth helper on, given that your KAuth actions are implemented in a totally insecure way. (Last I checked, the process running as the user passes an arbitrary file to the helper running as root, defeating the whole purpose of finegrained PolicyKit permissions. Give a user that PolicyKit permission and you essentially gave him root. Of course, it's not a problem if you stick to the default auth_admin policy, but if some local admin tries to change it, ouch!)


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/108711/#review26617
-----------------------------------------------------------


On Feb. 2, 2013, 8:27 a.m., Kevin Kofler wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://git.reviewboard.kde.org/r/108711/
> -----------------------------------------------------------
> 
> (Updated Feb. 2, 2013, 8:27 a.m.)
> 
> 
> Review request for kde-workspace, Christoph Feck and Oswald Buddenhagen.
> 
> 
> Description
> -------
> 
> Unfortunately, we cannot rely on the $PATH environment variable in KAuth helpers, because D-Bus activation clears it. So we have to use a reasonable default for the KStandardDirs::findExe search path, and actually use the return value of KStandardDirs::findExe in the calls to KProcess::execute.
> 
> This fixes things so hwclock and zic actually get found. See: https://bugzilla.redhat.com/show_bug.cgi?id=906854 . This got noticed in Fedora 18 because it does not always create /etc/localtime, so the fallback code operating on /etc/localtime triggered an error.
> 
> 
> Diffs
> -----
> 
>   kcontrol/dateandtime/helper.cpp 5a946d8 
> 
> Diff: http://git.reviewboard.kde.org/r/108711/diff/
> 
> 
> Testing
> -------
> 
> Builds against at least 4.10.0 and 4.9.5.
> 
> Works at runtime on Fedora 18, see: https://bugzilla.redhat.com/show_bug.cgi?id=906854#c12 (The reporter encountered another issue, apparently because ktimezoned also misbehaves when /etc/localtime is absent, but at least this particular issue is confirmed fixed.)
> 
> 
> Thanks,
> 
> Kevin Kofler
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-core-devel/attachments/20130204/b27899c1/attachment.htm>