Recursive dispatch (Re: [PATCH v2] server: Calculate remaining data size after a closure is processed)

[Pekka Paalanen]

On Wed, 3 Feb 2016 13:44:27 -0800
Bryce Harrington <bryce at osg.samsung.com> wrote:

> On Wed, Feb 03, 2016 at 01:37:21PM +0200, Pekka Paalanen wrote:
> > On Wed, 3 Feb 2016 10:52:07 +0000
> > Auke Booij <auke at tulcod.com> wrote:  
> > > On 3 February 2016 at 09:34, Jonas Ådahl <jadahl at gmail.com> wrote:  
> > > > On Wed, Feb 03, 2016 at 11:14:54AM +0200, Pekka Paalanen wrote:    
> > > >> On Wed, 3 Feb 2016 09:56:20 +0900
> > > >> "Jaeyoon Jung" <jaeyoon.jung at lge.com> wrote:  
> > > >> > > > If someone starts hardening libwayland against programmer errors,
> > > >> > > > I'd expect one thing to be to abort() on recursive dispatch. Can
> > > >> > > > you tell me why we should support recursive dispatch?    
> > > >> > >
> > > >> > > IMHO we should either do that immediately (before the 1.10 release) or
> > > >> > > revert this patch now.  Not that anything is wrong with the patch - it
> > > >> > > doesn't introduce bugs in libwayland...
> > > >> > >
> > > >> > > Previously recursive dispatch was impossible, with this patch it's
> > > >> > > possible.  The patch was landed to allow recursive dispatch to work,
> > > >> > > and it's obvious someone's about to start using it.    

> > > >> > > So, with the release coming so quickly, I think we should either
> > > >> > > decide whether recursive dispatch is legal or illegal before the
> > > >> > > release, or revert the patch (thus re-breaking recursive dispatch) and
> > > >> > > worry about it for 1.11.    
> > > >>
> > > >> I would declare recursive dispatch as illegal. Does anyone object?    
> > > >
> > > > I don't object declaring it illegal in libwayland-server since it was
> > > > previously broken, but doing so in libwayland-client I don't think we
> > > > should (at least yet) since it is used here and there in various clients
> > > > already.    
> > 
> > Ok, let's do it for server since that's what we are looking at right
> > now. (Warnings and aborts would be implemented later.)
> > 
> > Client side is indeed more complicated, since it provably works
> > somewhat, we've had it in some Weston demos even, that were since fixed
> > AFAIK.  
> 
> Is there any chance that a client could make use of this to overload the
> server?  If there is, would that imply some security implications that
> we need to account for?  It is probably wiser to close such holes before
> releasing, even if it might risk delaying it.

Hi,

these issues with libwayland-client usage cannot cause new harm to
compositors.

However, if a compositor is dispatching recursively, does it make it
more vulnerable to maliciously crafted requests? Perhaps, but I would
think the impact is tiny.

Is it worth making those compositors crash right now whether they are
being attacked or not? That's a good question. Before the patch
referenced in this thread, clients would randomly get disconnected, and
right now things would seem to work with recursive dispatch in a
compositor, but we should have the documentation in place to say it's a
bad idea.

In any case, we cannot fix any potential vulnerability on this topic in
libwayland-server (or -client), we can only make the respective
programs crash at most.


Thanks,
pq
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 811 bytes
Desc: OpenPGP digital signature
URL: <http://lists.freedesktop.org/archives/wayland-devel/attachments/20160204/204aa95c/attachment-0001.sig>