Subject: Re: [boost] [Stacktrace] review, please stop discussing non-Stacktrace issues
From: Andrey Semashev (andrey.semashev_at_[hidden])
Date: 2016-12-19 13:21:27
On 12/19/16 20:04, Peter Dimov wrote:
> Andrey Semashev wrote:
>> Suppose someone placed $HOME/bin/addr2line of the following content:
> That's only going to work if $HOME/bin is on the path before /usr/bin,
> which seems not very prudent from a security perspective. The user can
> type 'addr2line' (or anything else in /usr/bin such as 'ls') himself,
> after all. Hello root. So it's not that easy.
Well, $HOME/bin is before /usr/bin on my system. And I suppose, it is
expected to be looked up first, precisely to be able to override system
binaries with your own. Similar to how /usr/local/bin is expected to be
looked up before /usr/bin.
> In general, if the attacker has write access to a directory in $PATH,
> things are already not very secure. This also applies to Windows,
> because $PATH is searched for DLLs, although it has very low priority,
> so you need to find a DLL that the program attempts to load but isn't
> present in the system directories.
I think, a process could also modify the system-wide PATH variable, at
least on some Windows versions. IIRC, some installers (Visual Studio?
Intel compiler? I can't remember) did that at some point. I don't know
if that's the case now.
> That said, Stacktrace should probably not use $PATH at all for locating
> its helper process. On POSIX, execvp is not async safe anyway, so using
> /usr/bin/addr2line directly may be better.
That is somewhat better. It does exclude PATH from the lookup, that's
for sure. But consider that the application could use a custom build of
Boost and be installed in /opt. What if the application also wants to
use a custom addr2line from its /opt/my_app/bin? It's possible to patch
the path in Boost.Stacktrace, but that adds some maintenance burden of
having to maintain the patch. A config macro perhaps? A runtime option?
The full path does not solve the problem of setting up environment for
the process though. E.g. I don't have a way to drop privileges of root
before executing addr2line. Environment setup is also problematic - one
has to configure and restore environment before and after every call of
Boost.Stacktrace that executes a process.
> And on Windows, the helper
> would generally be installed along with the program - it won't be a
> system utility such as addr2line. In which case it would be spawned from
> the directory of the program using a full path.
That imposes a requirement on the user's application layout. I'd say, it
should be configurable as well. It should probably be a runtime
configuration in order to support portable applications (i.e. those not
requiring installation in a particular directory).
Security issues aside, there is also a matter of controlling the child
process. What if it hangs? Or crashes? How does it affect the master
application? What if the master crashes (i.e. how can you ensure you
don't leave a zombie process)? What will you do if you exhaust the limit
of running processes in the system (i.e. if the app itself is not able
to spawn its processes because of a bunch of running addr2line)? Then,
there are performance implications of running a process for each
function call. These kind of questions are difficult to answer in a
generic library, which is also not focused on process management.
I'm really not convinced that I have to execute a process to be able to
decode a stacktrace. That may be the only way to do that from a signal
handler, but in a general context that is really not the expected
behavior of the library. IMO, the signal handler case is special enough
to warrant a special API for that, and it may involve forking and
executing a process. That API should likely include the necessary means
to configure the process before `exec`ing the new process, including the
full path, permissions, a running timeout and what not. For other cases
an in-process solution should be available.
Boost list run by bdawes at acm.org, gregod at cs.rpi.edu, cpdaniel at pacbell.net, john at johnmaddock.co.uk