From 96a45b98b1f4475f38eb5ae6353f8fc6a6ab2117 Mon Sep 17 00:00:00 2001 From: AnkleBreaker Cowork Date: Mon, 20 Jul 2026 18:31:33 +0200 Subject: [PATCH] [FIX] Native audit hardening : ELF NOTE bounds, atomic re-entry guard, TLS verify, breadcrumb scrub, version Production-audit findings (no API/ABI change): - read_build_id: bounds-check name+descriptor vs segment end + forward-progress guard (OOB read on a truncated/hostile PT_NOTE at install). - signal re-entry guard: std::atomic_flag::test_and_set (async-signal-safe + cross-thread) replacing a non-atomic volatile sig_atomic_t (dual-thread fault could corrupt the dump). - transport_curl: set CURLOPT_SSL_VERIFYPEER/VERIFYHOST explicitly (don't inherit host defaults). - BreadcrumbRing::clear(): scrub slot strings on consent-revoke/GDPR reset. - tombstone_version() 0.8.0 -> 0.9.1; header docblock now describes the shipped opt-in handler. v0.9.1. --- CHANGELOG.md | 17 +++++++++++++++++ CMakeLists.txt | 2 +- include/tombstone/tombstone.h | 10 ++++++---- src/breadcrumb_ring.cpp | 7 +++++++ src/native_crash.cpp | 30 +++++++++++++++++++++--------- src/tombstone_api.cpp | 2 +- src/transport_curl.cpp | 5 +++++ 7 files changed, 58 insertions(+), 15 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 286b259..106cd94 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,23 @@ All notable changes to the Tombstack Native SDK (the `tombstone_*` C ABI and the `tombstone` library name are stable — Tombstack is the product name). +## [0.9.1] - 2026-07-20 + +### Hardening (production audit; no API/ABI change) + +- **Native crash handler — bounds-checked ELF NOTE parsing.** `read_build_id` now validates the note + name + descriptor against the segment end before reading, and guards forward progress, so a + truncated/hostile `PT_NOTE` can't drive an out-of-bounds read at handler install. +- **Native crash handler — atomic re-entry guard.** The double-fault guard is now + `std::atomic_flag::test_and_set` (async-signal-safe AND cross-thread) instead of a + read-then-write `volatile sig_atomic_t`, so two threads faulting at once can't both write the dump. +- **TLS verification asserted.** The libcurl transport now sets `CURLOPT_SSL_VERIFYPEER`/`VERIFYHOST` + explicitly instead of inheriting the integrator's defaults. +- **Breadcrumb memory scrub on consent revoke / GDPR reset.** `BreadcrumbRing::clear()` now clears the + slot strings, not just the head/count, so pre-revoke breadcrumb text doesn't linger in process memory. +- **Version string fixed** — `tombstone_version()` now reports the real version (was stuck at `0.8.0`); + header docblock updated to describe the shipped opt-in native handler. + ## [0.9.0] - 2026-07-20 ### Added — in-process native crash handler (EXPERIMENTAL, opt-in; Linux/Android) diff --git a/CMakeLists.txt b/CMakeLists.txt index cab7648..f9fb155 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -1,5 +1,5 @@ cmake_minimum_required(VERSION 3.16) -project(tombstone VERSION 0.9.0 LANGUAGES C CXX) +project(tombstone VERSION 0.9.1 LANGUAGES C CXX) option(TOMBSTONE_BUILD_STATIC "Also build a static tombstone library" OFF) option(TOMBSTONE_BUILD_EXAMPLES "Build the example programs" ON) diff --git a/include/tombstone/tombstone.h b/include/tombstone/tombstone.h index d729d8d..3dec3b6 100644 --- a/include/tombstone/tombstone.h +++ b/include/tombstone/tombstone.h @@ -13,10 +13,12 @@ * opaque type reserved for a future multi-instance API). Double-init and * use-after-shutdown return result codes; they are never undefined behavior. * - * Crash capture scope (v0.x): this SDK REPORTS crashes you hand it - * (tombstone_report_crash) and detects unclean shutdowns across launches. - * It does NOT install signal/SEH handlers or write minidumps — that arrives - * in Phase 2 via a sentry-native/Crashpad fork (see README roadmap). + * Crash capture scope: this SDK REPORTS crashes you hand it + * (tombstone_report_crash) and detects unclean shutdowns across launches. As of + * v0.9 it can ALSO install an in-process async-signal-safe native crash handler + * (opt-in via options.enable_native_crash_handler, default off; Linux/Android) — + * see that field. Full stack unwinding (Breakpad) + Windows SEH + iOS Mach + * remain on the roadmap (see README). * * Pre-init capture (v0.7): tombstone_add_breadcrumb / tombstone_track_event / * tombstone_track_metric / tombstone_set_user / tombstone_set_environment / diff --git a/src/breadcrumb_ring.cpp b/src/breadcrumb_ring.cpp index a98b177..2f32f2d 100644 --- a/src/breadcrumb_ring.cpp +++ b/src/breadcrumb_ring.cpp @@ -44,6 +44,13 @@ void BreadcrumbRing::clear() { const std::lock_guard lock(mutex_); head_ = 0; count_ = 0; + // Scrub slot memory too: clear() is called on consent revoke / GDPR reset, so pre-revoke + // breadcrumb text (potentially PII) must not linger in the process until later overwrites. + for (auto &slot : slots_) { + slot.ts_iso.clear(); + slot.level.clear(); + slot.message.clear(); + } } } // namespace tombstone diff --git a/src/native_crash.cpp b/src/native_crash.cpp index 9fbab93..5b22fb9 100644 --- a/src/native_crash.cpp +++ b/src/native_crash.cpp @@ -33,6 +33,7 @@ #include #include #include +#include #include #include #include @@ -128,11 +129,15 @@ struct HandlerState { int dump_fd{-1}; struct sigaction old_actions[kSignalCount]; bool installed{false}; - volatile sig_atomic_t in_handler{0}; // re-entrance / double-fault guard }; HandlerState g_state; +// Re-entrance / double-fault guard. `atomic_flag::test_and_set` is the ONE lock-free op guaranteed +// async-signal-safe AND atomic across threads (a plain `volatile sig_atomic_t` read-then-write is +// not a CAS — two threads faulting at once could both pass and corrupt the single dump fd). +std::atomic_flag g_in_handler = ATOMIC_FLAG_INIT; + // Fixed alternate signal stack (glibc 2.34+ made SIGSTKSZ a runtime value that // can't size an array): 64 KiB is comfortably above SIGSTKSZ on every target. char g_alt_stack[65536]; @@ -155,15 +160,22 @@ void read_build_id(const ElfW(Phdr) & phdr, std::uintptr_t base, ModuleEntry &en while (reinterpret_cast(note) + sizeof(ElfW(Nhdr)) <= end) { const char *name = reinterpret_cast(note) + sizeof(ElfW(Nhdr)); const char *desc = name + ((note->n_namesz + 3) & ~3u); - if (note->n_type == NT_GNU_BUILD_ID && note->n_namesz == 4 && + // Bounds: a truncated / hostile NOTE must never drive an out-of-bounds read. `desc` past the + // segment end, or the "GNU" name / the descriptor overrunning `end`, aborts the walk. + if (desc > end) break; + if (note->n_type == NT_GNU_BUILD_ID && note->n_namesz == 4 && name + 4 <= end && std::memcmp(name, "GNU", 3) == 0) { std::size_t len = note->n_descsz; if (len > kMaxBuildIdBytes) len = kMaxBuildIdBytes; - std::memcpy(entry.build_id, desc, len); - entry.build_id_len = len; + if (desc + len <= end) { + std::memcpy(entry.build_id, desc, len); + entry.build_id_len = len; + } return; } - note = reinterpret_cast(desc + ((note->n_descsz + 3) & ~3u)); + const char *next = desc + ((note->n_descsz + 3) & ~3u); + if (next <= reinterpret_cast(note)) break; // no forward progress → stop + note = reinterpret_cast(next); } } @@ -325,13 +337,13 @@ void chain_old(int sig, siginfo_t *info, void *ucontext) { } extern "C" void tombstone_signal_handler(int sig, siginfo_t *info, void *ucontext) { - // Re-entrance / double-fault guard: if a second fatal signal arrives while - // we're dumping, don't recurse — hand straight to the old handler. - if (g_state.in_handler != 0) { + // Re-entrance / double-fault guard (atomic test-and-set): a second fatal signal — including one on + // another thread while we're dumping — must not recurse or share the fd; hand straight to the old + // handler. Never cleared: a crash is terminal. + if (g_in_handler.test_and_set()) { chain_old(sig, info, ucontext); return; } - g_state.in_handler = 1; const int fd = g_state.dump_fd; if (fd >= 0) { diff --git a/src/tombstone_api.cpp b/src/tombstone_api.cpp index 40eeb9b..305385a 100644 --- a/src/tombstone_api.cpp +++ b/src/tombstone_api.cpp @@ -12,7 +12,7 @@ namespace { -constexpr const char *sdk_version = "0.8.0"; +constexpr const char *sdk_version = "0.9.1"; // The process-wide client. Entry points snapshot the shared_ptr under the // mutex and call outside it, so a long flush() neither blocks other calls nor diff --git a/src/transport_curl.cpp b/src/transport_curl.cpp index f593bd2..649fbc3 100644 --- a/src/transport_curl.cpp +++ b/src/transport_curl.cpp @@ -104,6 +104,11 @@ HttpResponse perform(CURL *handle, SdkLog &sdk_log) { curl_easy_setopt(handle, CURLOPT_HEADERDATA, &retry_after); curl_easy_setopt(handle, CURLOPT_NOSIGNAL, 1L); curl_easy_setopt(handle, CURLOPT_FOLLOWLOCATION, 0L); + // Assert TLS verification explicitly rather than inheriting the integrator's libcurl defaults: + // this SDK ships a Bearer ingest token to a remote endpoint and must not accept an untrusted or + // MITM'd cert even if the host build flipped the defaults. (libcurl defaults to these today.) + curl_easy_setopt(handle, CURLOPT_SSL_VERIFYPEER, 1L); + curl_easy_setopt(handle, CURLOPT_SSL_VERIFYHOST, 2L); const CURLcode code = curl_easy_perform(handle); HttpResponse response;