Review Request 126524: Fixed custom wallpaper being not respected by sddm.

[David Edmundson]

> On Dec. 27, 2015, 4:37 p.m., David Edmundson wrote:
> > sddmauthhelper.cpp, line 74
> > <https://git.reviewboard.kde.org/r/126524/diff/1/?file=426085#file426085line74>
> >
> >     Ideally we should check the *calling* user can read this file, as you technically have a security bug.
> >     
> >     Otherwise I could just select /etc/shadow as my background and suddenly it's available world readable.
> >     
> >     A distro/admin could theoretically set polkit up to allow any users to change the SDDM wallpaper. though TBH it'd never happen.
> >     
> >     Polkit-Qt has that information available. KAuth does not seem to publicly.
> 
> Joshua Noack wrote:
>     Not sure if I understand you correctly... 
>     In addition to the KAuth dialog where the user needs to authenticate, a check if the user can read the file should be added?
>     Shouldn't the file chooser restrict the user here in the first place?

it needs to happen inside the helper, not on the client side.

>From a terminal I can manually start the sddm helper saying I want to set /etc/shadow as the background.

The helper will then check if the user is permitted to set the SDDM wallpaper in policykit, which is up to the sysadmin's policy
If that returns true, the helper will procede to copy the file as root.

That's a security hole as it would potentially allow me to read any file.


- David


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/126524/#review90132
-----------------------------------------------------------


On Dec. 28, 2015, 3:44 p.m., Joshua Noack wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/126524/
> -----------------------------------------------------------
> 
> (Updated Dec. 28, 2015, 3:44 p.m.)
> 
> 
> Review request for Plasma and David Edmundson.
> 
> 
> Repository: sddm-kcm
> 
> 
> Description
> -------
> 
> For some reason sddm cannot handle absolute file paths to wallpapers
> and also needs the wallpaper to be readable by others.
> 
> This is fixed by copying the wallpaper to the root directory of the
> selected theme.
> 
> On save the sddmauthhelper copies the background from the absolute path
> into the theme directory and sets the "background" key of the
> theme.user.conf to the copied file. If previously a different background was
> set it is removed beforehand.
> 
> 
> Diffs
> -----
> 
>   sddmauthhelper.cpp 648b24c77e7570641d454fca9d121709a622bc36 
>   src/themeconfig.cpp bdd6dd29fd8eb052e2f2b2239b0c46ebbebec88c 
> 
> Diff: https://git.reviewboard.kde.org/r/126524/diff/
> 
> 
> Testing
> -------
> 
> Copies and removes backgrounds as intended.
> The wallpaper is shown in sddm.
> 
> 
> Thanks,
> 
> Joshua Noack
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/plasma-devel/attachments/20151228/d89a969e/attachment.html>