[Wayland-bugs] [Bug 84040] New: weston-launch is setuid, so it should handle the environment in a paranoid way

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=84040

          Priority: medium
            Bug ID: 84040
          Assignee: wayland-bugs at lists.freedesktop.org
           Summary: weston-launch is setuid, so it should handle the
                    environment in a paranoid way
          Severity: normal
    Classification: Unclassified
                OS: All
          Reporter: simon.mcvittie at collabora.co.uk
          Hardware: Other
            Status: NEW
           Version: unspecified
         Component: weston
           Product: Wayland

Similar to Bug #83849, weston-launch runs with elevated privileges (it is
setuid), so it needs to be careful not to trust its environment. It is linked
to arbitrary libraries (via libpam if nothing else), and should not assume that
those libraries are all designed to be setuid-safe - most libraries aren't.

(See, e.g., Bug #52202 in libdbus, which was not designed to be setuid-safe,
and had that bolted on as an afterthought when it became clear that people were
using it in an unsupported way anyway.)

It is possible that weston-launch is actually completely OK - it does do a
clearenv() before invoking PAM, and the rest of its code seems to be just libc
and libsystemd.

However, it would be more obviously correct (the best kind of correctness for
security-sensitive code) if it behaved more like this pseudocode:

    original_environ = deep-copy of environ

    clearenv()
    foreach (whitelist of known-safe variables, e.g. TERM):
        if (variable is in original_environ and its value is safe):
            copy it to new environment

    ... do stuff with privileges ...

    if (on code path where we drop privileges):
        fork() or whatever
        if (in child process):
            drop privileges
            (optionally) put original_environ back
            exec(thing that must run as original user)

When I say "its value is safe" I mean a check specific to that variable: the
more strict its consumers are, the more lenient you can be. For instance,
pkexec does this:

SHELL: must be in /etc/shells
XAUTHORITY: must not contain % or ..
LANG, LINGUAS, LANGUAGE, LC_*, TERM, COLORTERM: must not contain /, % or ..

which seems reasonable.

-- 
You are receiving this mail because:
You are the assignee for the bug.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.freedesktop.org/archives/wayland-bugs/attachments/20140918/578d1247/attachment.html>