Review Request 128219: No longer allow installing to generic data folder because of security hole.

[Jeremy Whiting]

> On June 17, 2016, 1:36 a.m., David Faure wrote:
> > src/core/installation.cpp, line 365
> > <https://git.reviewboard.kde.org/r/128219/diff/1/?file=469097#file469097line365>
> >
> >     Should this code get support for "appdata" then?
> >      (typically share/kmyapp)
> >      
> >     Otherwise I don't see where application data would get installed anymore.
> >     
> >     Am I right that there must be lots of apps using "data" right now, for lack of "appdata" support?
> >     Surely not every app using knewstuff, is using it for "tmp" or "config" files....

I have a build of most of the "official" kde applications here. Not all of the possible .knsrc files by any means, but a good selection most likely. Looking at what I have here all uses of StandardResource were either tmp or wallpapers. Most applications use TargetDir instead and specify a path within appdata. apps/kvtml, color-schemes, cantor/examples etc. I couldn't see any at all here that are using StandardResource=data directly.

I guess I should do a more thorough search on lxr.kde.org though.

Ok, doing that https://lxr.kde.org/search?_filestring=.knsrc&_string=StandardResource&_casesensitive=1 shows all StandardResource= being tmp or wallpaper. No uses of "data" at all.


> On June 17, 2016, 1:36 a.m., David Faure wrote:
> > src/core/installation.cpp, line 366
> > <https://git.reviewboard.kde.org/r/128219/diff/1/?file=469097#file469097line366>
> >
> >     API misuse is normally rewarded with a q[C]Warning rather than a q[C]Debug.
> >     
> >     The message should also mention what to use instead (depending on the result of the discussion in the previous comment).

Yep, I'll change to qCWarning, np and mention what to use instead.


> On June 17, 2016, 1:36 a.m., David Faure wrote:
> > src/core/installation.cpp, line 379
> > <https://git.reviewboard.kde.org/r/128219/diff/1/?file=469097#file469097line379>
> >
> >     There are of course other values for targetDirectory which would create problems.
> >     - "//"
> >     - "./"
> >     - "../etc"
> >     - and so on
> >     
> >     But this is a setting written by the app developer, not by the person uploading knewstuff data, so we can assume no malicious intention, right?

Yes only application developer. Or end user if they want to tweak the .knsrc files by hand to introduce a security vulnerability. Though if they wanted to do that there are much easier ways to do it.


- Jeremy


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


On June 16, 2016, 7:55 p.m., Jeremy Whiting wrote:
> 
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/128219/
> -----------------------------------------------------------
> 
> (Updated June 16, 2016, 7:55 p.m.)
> 
> 
> Review request for KDE Frameworks, David Faure and Richard Moore.
> 
> 
> Repository: knewstuff
> 
> 
> Description
> -------
> 
> When an application uses TargetDir=/ or StandardResource=data give a warning on the terminal and don't use the chosen location.
> 
> 
> Diffs
> -----
> 
>   src/core/installation.cpp cbd0653 
> 
> Diff: https://git.reviewboard.kde.org/r/128219/diff/
> 
> 
> Testing
> -------
> 
> No testing done yet, will write a unit test of some kind if this is the right direction.
> 
> 
> Thanks,
> 
> Jeremy Whiting
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20160617/6dba191d/attachment.html>