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.