On 2018/02/23 17:06, Theo Buehler wrote:
> On Fri, Feb 23, 2018 at 09:48:25AM +0000, Stuart Henderson wrote:
> > On 2018/02/23 09:45, Klemens Nanni wrote:
> > > > > -+ if (pledge("stdio rpath proc exec", NULL) == -1)
> > > > > -+ err(1, "pledge");
> > > > > ++
> > > > > ++ /* allow prot_exec if icons are used */
> > > > > ++ if (settings.icon_position != icons_off) {
> > > > > ++ if (pledge("stdio rpath proc exec prot_exec", NULL)
> > > > > == -1)
> > > > > ++ err(1, "pledge");
> > > > > ++ } else {
> > > > > ++ if (pledge("stdio rpath proc exec", NULL) == -1)
> > > > > ++ err(1, "pledge");
> > > > > ++ }
> >
> > Generally OK with the update but regarding the pledge change:
> >
> > Honestly I'd keep it simple. It's not like there's a super-tight pledge
> > already - it has unrestricted exec. Rather than adding a conditional to
> > the pledge I think you might as well just have the single check and add
> > prot_exec to the pledge. CC'ing tb@ for a second opinion though :)
> >
> > I'd probably commit as 2-part so the commit history is clear. 1, fix
> > pledge in the existing port (since the problem is not new), 2, do the
> > update.
> >
>
> I'm not convinced about the correctness of this apart from some people
> assuring that it "seems to work". As far as I know, allowing icons
> results in calls out to GdkPixbuf that in turn dlopen some library I
> don't know at all. Has anyone audited this to make sure that the list
> of of pledges is complete? I've not seen such an analysis.
>
> Lacking this, I would suggest to be conservative and pledge only
> condititonally on "settings.icon_position == icons.off" and run
> unpledged otherwise.
Thanks for looking - OK yes, that makes sense to me.
