<html>
<head>
<base href="https://bugs.freedesktop.org/" />
</head>
<body><table border="1" cellspacing="0" cellpadding="8">
<tr>
<th>Priority</th>
<td>medium
</td>
</tr>
<tr>
<th>Bug ID</th>
<td><a class="bz_bug_link
bz_status_NEW "
title="NEW --- - weston-launch is setuid, so it should handle the environment in a paranoid way"
href="https://bugs.freedesktop.org/show_bug.cgi?id=84040">84040</a>
</td>
</tr>
<tr>
<th>Assignee</th>
<td>wayland-bugs@lists.freedesktop.org
</td>
</tr>
<tr>
<th>Summary</th>
<td>weston-launch is setuid, so it should handle the environment in a paranoid way
</td>
</tr>
<tr>
<th>Severity</th>
<td>normal
</td>
</tr>
<tr>
<th>Classification</th>
<td>Unclassified
</td>
</tr>
<tr>
<th>OS</th>
<td>All
</td>
</tr>
<tr>
<th>Reporter</th>
<td>simon.mcvittie@collabora.co.uk
</td>
</tr>
<tr>
<th>Hardware</th>
<td>Other
</td>
</tr>
<tr>
<th>Status</th>
<td>NEW
</td>
</tr>
<tr>
<th>Version</th>
<td>unspecified
</td>
</tr>
<tr>
<th>Component</th>
<td>weston
</td>
</tr>
<tr>
<th>Product</th>
<td>Wayland
</td>
</tr></table>
<p>
<div>
<pre>Similar to <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - xserver should sanitize/whitelist the environment"
href="show_bug.cgi?id=83849">Bug #83849</a>, 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., <a class="bz_bug_link
bz_status_NEW "
title="NEW --- - document that setuid executables must clear their environment before using libdbus"
href="show_bug.cgi?id=52202">Bug #52202</a> 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.</pre>
</div>
</p>
<hr>
<span>You are receiving this mail because:</span>
<ul>
<li>You are the assignee for the bug.</li>
</ul>
</body>
</html>