Bake an absolute path for addr2line into the library to prevent against path injection (#19)

This commit is contained in:
Jeremy Rifkin 2023-07-23 23:01:20 -04:00 committed by GitHub
parent e1c7657a3e
commit 43383228e7
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 101 additions and 31 deletions

View File

@ -67,6 +67,11 @@ option(CPPTRACE_UNWIND_WITH_NOTHING "" OFF)
option(CPPTRACE_DEMANGLE_WITH_CXXABI "" OFF)
option(CPPTRACE_DEMANGLE_WITH_NOTHING "" OFF)
set(CPPTRACE_BACKTRACE_PATH "" CACHE STRING "Path to backtrace.h, if the compiler doesn't already know it. Check /usr/lib/gcc/x86_64-linux-gnu/*/include.")
set(CPPTRACE_HARD_MAX_FRAMES "" CACHE STRING "Hard limit on unwinding depth. Default is 100.")
set(CPPTRACE_ADDR2LINE_PATH "" CACHE STRING "Absolute path to the addr2line executable you want to use.")
option(CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH "" OFF)
option(CPPTRACE_BUILD_TEST "" OFF)
option(CPPTRACE_BUILD_DEMO "" OFF)
option(CPPTRACE_BUILD_TEST_RDYNAMIC "" OFF)
@ -75,9 +80,6 @@ option(CPPTRACE_BUILD_SPEEDTEST "" OFF)
option(CPPTRACE_BUILD_SPEEDTEST_DWARF4 "" OFF)
option(CPPTRACE_BUILD_SPEEDTEST_DWARF5 "" OFF)
set(CPPTRACE_BACKTRACE_PATH "" CACHE STRING "Path to backtrace.h, if the compiler doesn't already know it. Check /usr/lib/gcc/x86_64-linux-gnu/*/include.")
set(CPPTRACE_HARD_MAX_FRAMES "" CACHE STRING "Hard limit on unwinding depth. Default is 100.")
if(NOT "${CPPTRACE_BACKTRACE_PATH}" STREQUAL "")
# quotes used over <> because of a macro substitution issue where
# </usr/lib/gcc/x86_64-linux-gnu/12/include/backtrace.h>
@ -276,6 +278,23 @@ if(CPPTRACE_GET_SYMBOLS_WITH_LIBDL)
endif()
if(CPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE)
# set(CPPTRACE_ADDR2LINE_PATH "" CACHE STRING "Absolute path to the addr2line executable you want to use.")
# option(CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH "" OFF)
if(CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH)
target_compile_definitions(cpptrace PUBLIC CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH)
else()
if("${CPPTRACE_ADDR2LINE_PATH}" STREQUAL "")
if(APPLE)
find_program(CPPTRACE_ADDR2LINE_PATH_FINAL atos PATHS ENV PATH REQUIRED)
else()
find_program(CPPTRACE_ADDR2LINE_PATH_FINAL addr2line PATHS ENV PATH REQUIRED)
endif()
else()
set(CPPTRACE_ADDR2LINE_PATH_FINAL "${CPPTRACE_ADDR2LINE_PATH}")
endif()
message(STATUS "Cpptrace: Using ${CPPTRACE_ADDR2LINE_PATH_FINAL} for addr2line path")
target_compile_definitions(cpptrace PUBLIC CPPTRACE_ADDR2LINE_PATH="${CPPTRACE_ADDR2LINE_PATH_FINAL}")
endif()
target_compile_definitions(cpptrace PUBLIC CPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE)
if(UNIX)
target_link_libraries(cpptrace PRIVATE dl)

View File

@ -185,6 +185,10 @@ can hold addresses for 100 frames (beyond the `skip` frames). This is configurab
*: Requires installation
Note for addr2line: By default cmake will resolve an absolute path to addr2line to bake into the library. This path can
be configured with `CPPTRACE_ADDR2LINE_PATH`, or `CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH` can be used to have the library
search the system path for `addr2line` at runtime. This is not the default to prevent against path injection attacks.
**Demangling**
Lastly, depending on other back-ends used a demangler back-end may be needed. A demangler back-end is not needed when
@ -215,24 +219,29 @@ more back-ends can be added. Ideally this library can "just work" on systems, wi
Summary of all library configuration options:
Back-ends:
- `CPPTRACE_FULL_TRACE_WITH_LIBBACKTRACE`
- `CPPTRACE_FULL_TRACE_WITH_STACKTRACE`
- `CPPTRACE_GET_SYMBOLS_WITH_LIBBACKTRACE`
- `CPPTRACE_GET_SYMBOLS_WITH_LIBDL`
- `CPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE`
- `CPPTRACE_GET_SYMBOLS_WITH_DBGHELP`
- `CPPTRACE_GET_SYMBOLS_WITH_NOTHING`
- `CPPTRACE_UNWIND_WITH_UNWIND`
- `CPPTRACE_UNWIND_WITH_EXECINFO`
- `CPPTRACE_UNWIND_WITH_WINAPI`
- `CPPTRACE_UNWIND_WITH_NOTHING`
- `CPPTRACE_DEMANGLE_WITH_CXXABI`
- `CPPTRACE_DEMANGLE_WITH_NOTHING`
- `CPPTRACE_FULL_TRACE_WITH_LIBBACKTRACE=On/Off`
- `CPPTRACE_FULL_TRACE_WITH_STACKTRACE=On/Off`
- `CPPTRACE_GET_SYMBOLS_WITH_LIBBACKTRACE=On/Off`
- `CPPTRACE_GET_SYMBOLS_WITH_LIBDL=On/Off`
- `CPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE=On/Off`
- `CPPTRACE_GET_SYMBOLS_WITH_DBGHELP=On/Off`
- `CPPTRACE_GET_SYMBOLS_WITH_NOTHING=On/Off`
- `CPPTRACE_UNWIND_WITH_UNWIND=On/Off`
- `CPPTRACE_UNWIND_WITH_EXECINFO=On/Off`
- `CPPTRACE_UNWIND_WITH_WINAPI=On/Off`
- `CPPTRACE_UNWIND_WITH_NOTHING=On/Off`
- `CPPTRACE_DEMANGLE_WITH_CXXABI=On/Off`
- `CPPTRACE_DEMANGLE_WITH_NOTHING=On/Off`
General:
- `CPPTRACE_BACKTRACE_PATH`: Path to libbacktrace backtrace.h, needed when compiling with clang
- `CPPTRACE_HARD_MAX_FRAMES`: Some back-ends write to a fixed-size buffer. This is the size of that buffer. Default is
`100`.
Back-end configuration:
- `CPPTRACE_BACKTRACE_PATH=<string>`: Path to libbacktrace backtrace.h, needed when compiling with clang
- `CPPTRACE_HARD_MAX_FRAMES=<number>`: Some back-ends write to a fixed-size buffer. This is the size of that buffer.
Default is `100`.
- `CPPTRACE_ADDR2LINE_PATH=<string>`: Specify the absolute path to the addr2line binary for cpptrace to invoke. By
default the config script will search for a binary and use that absolute path (this is to prevent against path
injection).
- `CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH=On/Off`: Specifies whether cpptrace should let the system search the PATH
environment variable directories for the binary.
Testing:
- `CPPTRACE_BUILD_TEST` Build a small test program

View File

@ -4,7 +4,7 @@ status=0
while read f
do
echo checking $f
flags="-DCPPTRACE_FULL_TRACE_WITH_LIBBACKTRACE -DCPPTRACE_FULL_TRACE_WITH_STACKTRACE -DCPPTRACE_GET_SYMBOLS_WITH_LIBBACKTRACE -DCPPTRACE_GET_SYMBOLS_WITH_LIBDL -DCPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE -DCPPTRACE_GET_SYMBOLS_WITH_NOTHING -DCPPTRACE_UNWIND_WITH_EXECINFO -DCPPTRACE_UNWIND_WITH_UNWIND -DCPPTRACE_UNWIND_WITH_NOTHING -DCPPTRACE_DEMANGLE_WITH_CXXABI -DCPPTRACE_DEMANGLE_WITH_NOTHING"
flags="-DCPPTRACE_FULL_TRACE_WITH_LIBBACKTRACE -DCPPTRACE_FULL_TRACE_WITH_STACKTRACE -DCPPTRACE_GET_SYMBOLS_WITH_LIBBACKTRACE -DCPPTRACE_GET_SYMBOLS_WITH_LIBDL -DCPPTRACE_GET_SYMBOLS_WITH_ADDR2LINE -DCPPTRACE_GET_SYMBOLS_WITH_NOTHING -DCPPTRACE_UNWIND_WITH_EXECINFO -DCPPTRACE_UNWIND_WITH_UNWIND -DCPPTRACE_UNWIND_WITH_NOTHING -DCPPTRACE_DEMANGLE_WITH_CXXABI -DCPPTRACE_DEMANGLE_WITH_NOTHING -DCPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH"
clang-tidy $f -- -std=c++11 -Iinclude $@ $flags
ret=$?
if [ $ret -ne 0 ]; then

View File

@ -72,12 +72,18 @@ namespace cpptrace {
if(pid == 0) { // child
close(STDOUT_FILENO);
close(STDERR_FILENO); // atos --help writes to stderr
// TODO: path
#ifdef CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH
#if !IS_APPLE
execlp("addr2line", "addr2line", "--help", nullptr);
#else
execlp("atos", "atos", "--help", nullptr);
#endif
#else
#ifndef CPPTRACE_ADDR2LINE_PATH
#error "CPPTRACE_ADDR2LINE_PATH must be defined if CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH is not"
#endif
execl(CPPTRACE_ADDR2LINE_PATH, CPPTRACE_ADDR2LINE_PATH, "--help", nullptr);
#endif
_exit(magic);
}
int status;
@ -115,12 +121,31 @@ namespace cpptrace {
close(input_pipe.read_end);
close(input_pipe.write_end);
close(STDERR_FILENO); // TODO: Might be worth conditionally enabling or piping
// TODO: Prevent against path injection?
#ifdef CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH
#if !IS_APPLE
execlp("addr2line", "addr2line", "-e", executable.c_str(), "-f", "-C", "-p", nullptr);
#else
execlp("atos", "atos", "-o", executable.c_str(), nullptr);
#endif
#else
#ifndef CPPTRACE_ADDR2LINE_PATH
#error "CPPTRACE_ADDR2LINE_PATH must be defined if CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH is not"
#endif
#if !IS_APPLE
execl(
CPPTRACE_ADDR2LINE_PATH,
CPPTRACE_ADDR2LINE_PATH,
"-e",
executable.c_str(),
"-f",
"-C",
"-p",
nullptr
);
#else
execl(CPPTRACE_ADDR2LINE_PATH, CPPTRACE_ADDR2LINE_PATH, "-o", executable.c_str(), nullptr);
#endif
#endif
_exit(1); // TODO: Diagnostic?
}
internal_verify(write(input_pipe.write_end, addresses.data(), addresses.size()) != -1);
@ -218,7 +243,14 @@ namespace cpptrace {
if(!checked) {
// TODO: Popen is a hack. Implement properly with CreateProcess and pipes later.
checked = true;
#ifdef CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH
FILE* p = popen("addr2line --version", "r");
#else
#ifndef CPPTRACE_ADDR2LINE_PATH
#error "CPPTRACE_ADDR2LINE_PATH must be defined if CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH is not"
#endif
FILE* p = popen(CPPTRACE_ADDR2LINE_PATH " --version", "r");
#endif
if(p) {
has_addr2line = pclose(p) == 0;
}
@ -229,7 +261,17 @@ namespace cpptrace {
std::string resolve_addresses(const std::string& addresses, const std::string& executable) {
// TODO: Popen is a hack. Implement properly with CreateProcess and pipes later.
///fprintf(stderr, ("addr2line -e " + executable + " -fCp " + addresses + "\n").c_str());
#ifdef CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH
FILE* p = popen(("addr2line -e " + executable + " -fCp " + addresses).c_str(), "r");
#else
#ifndef CPPTRACE_ADDR2LINE_PATH
#error "CPPTRACE_ADDR2LINE_PATH must be defined if CPPTRACE_ADDR2LINE_SEARCH_SYSTEM_PATH is not"
#endif
FILE* p = popen(
(CPPTRACE_ADDR2LINE_PATH " -e " + executable + " -fCp " + addresses).c_str(),
"r"
);
#endif
std::string output;
constexpr int buffer_size = 4096;
char buffer[buffer_size];