New paths for Wayland sockets (Re: Enabling Android-style per application user ids)

[Carsten Haitzler]

On Fri, 3 Nov 2017 09:38:46 +0100 (CET) Jan Engelhardt <jengelh at inai.de> said:

> 
> On Friday 2017-11-03 08:33, Pekka Paalanen wrote:
> >> > >
> >> > > wl_display_connect() always attempts to contact a server socket living
> >> > > at ${XDG_RUNTIME_DIR}/${WAYLAND_DISPLAY}.
> >
> >> > Modifying the meaning of WAYLAND_DISPLAY environment variable to
> >> > support also absolute paths has been proposed before IIRC. Maybe
> >> > resurrecting that work could be a way forward? Can anyone see a problem
> >> > with that?
> >> 
> >> This would definitely work, so I don't object if this is the preference of
> >> other reviewers. I would prefer (for the reasons coming below) the
> >> /run/wayland/$WAYLAND_DISPLAY suggestion though.
> >> 
> >> One note about this: this would contain a subtle change in behavior to
> >> existing users of $WAYLAND_DISPLAY. If somebody sets
> >> WAYLAND_DISPLAY="/wayland-0" currently, this works okay. The concatenation
> >> logic in wl_display_connect() results in a string
> >> "/run/user/<uid>//wayland-0", which is valid despite the duplicate '/'. If
> >> $WAYLAND_DISPLAY is now examined for absolute-pathiness, the logic would
> >> probably see "/wayland-0" as an absolute path reference, and the connection
> >> attempt would fail.
> 
> So, for comparison, the X11 DISPLAY variable is an abstract identifier only.
> libX11 checks that $DISPLAY matches /^:[0-9]+$/ and rejects attempts to use
> e.g. "/../"-kind of injection attacks.
> 
> And I would think that the intent of WAYLAND_DISPLAY was very similar in that
> its creators saw it as an identifier rather than a path component. (It
> certainly has that _ring_ to it, given its proximity to X11's "DISPLAY" name.)
> 
> As such, WAYLAND_DISPLAY maybe should be hardened to reject any '/',
> and consequently, a new variable ("WAYLAND_SOCKET"?) be introduced that,
> if set, 1. overrides WAYLAND_DISPLAY, 2. decidedly allows path specs
> to the desired effect so as to use /run/wayland/wayland-N.

how would it be "attacked"? so if we're just talking regular user processes,
the user would be exploiting themselves. right? by telling the app to connect
to another socket and compositor (fake or real... does it matter?). if it's a
setuid process (privileged) then ... this env var will not exist when run (or
should not and any other environment that escalates privilege should set or
sanitize this env var). yes - they could let's say create a proxy cpmpositor
and listen to all i/o or even fake it... but they can do this withotu full
paths. just

export WAYLAND_DISPLAY=myproxy

and have $XDG_RUNTIME_DIR/myproxy be your "exploit" server proxy. it can be
done without full paths. xdg runtime dir is intended to be written and used by
the user... you could make this dir not writable by any user launching
processes... this would be entirely strange and probably break a lot of apps
that have ipc sockets. ?

i'm curious as to the attack vector.

> Summary of (individual) proposals follows.
> 
> >- modify WAYLAND_DISPLAY to support absolute paths which overrides
> >  any search paths
> 
>  - introduce new WAYLAND_SOCKET
>  - modify WAYLAND_DISPLAY to reject '/'
> 
> >- when the current socket search fails, add one more place to look
> >  in: /run/wayland/$WAYLAND_DISPLAY with the default "wayland-0" if
> >  WAYLAND_DISPLAY is not set.
> 
>  - WAYLAND_SOCKET always overrides WAYLAND_DISPLAY
>  - at most, automatic fallback if only if both WAYLAND_SOCKET and
>    WAYLAND_DISPLAY are unset
> 


-- 
------------- Codito, ergo sum - "I code, therefore I am" --------------
Carsten Haitzler - raster at rasterman.com