aboutsummaryrefslogtreecommitdiff
path: root/src/zenutil/process/exitwatcher.cpp
diff options
context:
space:
mode:
authorStefan Boberg <[email protected]>2026-05-04 16:46:03 +0200
committerGitHub Enterprise <[email protected]>2026-05-04 16:46:03 +0200
commit10d2a61fe1c848f44033e8450ff3a5ffa7f4322a (patch)
treeaa66c6a068b50d2390bdae5f857c7151f15e5a86 /src/zenutil/process/exitwatcher.cpp
parentTui picker fixes (#1027) (diff)
downloadarchived-zen-10d2a61fe1c848f44033e8450ff3a5ffa7f4322a.tar.xz
archived-zen-10d2a61fe1c848f44033e8450ff3a5ffa7f4322a.zip
zenhttp improvements (robustness / correctness) (#968)
A collection of security, correctness, and robustness fixes in `zenhttp` and `zencore` surfaced by security review. Most items are small, independent commits grouped here because they all tighten trust boundaries or fix UB along the same code paths. ## WebSocket protocol hardening (RFC 6455) - **Enforce the client-side mask bit**. Server-side frame loops now reject unmasked frames with close code 1002 per §5.1. Prevents HTTP intermediary smuggling. - **Validate control frames and RSV bits**. Fragmented control frames, oversized (>125 B) control payloads, and any non-zero RSV bit now fail the connection before allocation. - **Lower per-frame payload cap** from 256 MB → 4 MB. Bounds per-connection accumulator memory. - **Implement message fragmentation**. Continuation frames are coalesced and delivered as a single message; interleaved non-control frames close with 1002; assembled messages are capped at 4 MB (1009 on overflow). Previously partial fragments were delivered to handlers, bypassing payload validation. - **Parse the 101 handshake response properly** in `HttpWsClient`. Status-line, `Upgrade`, `Connection`, and `Sec-WebSocket-Accept` are now matched exactly rather than via substring searches against the full body. ## Auth / OIDC hardening - **Constant-time password compare** in `PasswordSecurity::IsAllowed` (closes a remote length/content timing oracle). Adds a shared `ConstantTimeEquals` helper. - **Harden Basic-auth header parsing**: trim trailing LWS, reject control bytes and DEL in the credential. - **OIDC discovery pinning**: require HTTPS (loopback exempt), verify `issuer` matches `BaseUrl`, require `token_endpoint` / `userinfo_endpoint` / `jwks_uri` to share origin with `BaseUrl`, reject empty `token_endpoint`. - **Restrict `POST /auth/oidc/refreshtoken`** to local-machine requests. Previously unauthenticated in default deployments — remote callers could evict or replace cached tokens. - **Stop logging OIDC provider response bodies** on refresh failure (IdPs echo `refresh_token` back in error bodies). - **Drop the unused `IdentityToken` field** from `OidcClient` / `OpenIdToken` so nothing in the tree accidentally trusts an unverified JWT. ## Auth state encryption migration - Add `AesGcm` AEAD primitive (BCrypt / OpenSSL backends, mbedTLS stubbed) and `CryptoRandom::Fill` CSPRNG helper in `zencore/crypto.h`. - Migrate authstate file from AES-256-CBC with a fixed IV to AES-GCM with a fresh 12-byte random nonce per write and the 4-byte `ZEN1` magic bound as AAD. Legacy-CBC files are transparently read once and rewritten in the new format. ## Filesystem / IO robustness - `IoBufferExtendedCore::Materialize` now checks `MAP_FAILED` on POSIX (was comparing to `nullptr`, which let the failure sentinel propagate into later reads and `munmap(MAP_FAILED, ...)`). - `IoBufferBuilder::MakeFromFile / MakeFromTemporaryFile`: close the FD/HANDLE on exception via a dismissable `ScopeGuard`; actually check the `fstat()` return value (previously used an uninitialized `FileSize`). - `ReadFromFileMaybe`: loop short reads, retry `EINTR`, chunk Windows `ReadFile` at `0xFFFFFFFF` bytes (fixes silent truncation of multi-GiB reads). - `WipeDirectory`: compare `FindFirstFileW` handle against `INVALID_HANDLE_VALUE` rather than `nullptr`. - `RemoveFileNative` (Linux/macOS): report non-`ENOENT` stat failures via the `std::error_code` out-param and stop reading `st_mode` after a failed stat. ## Buffer / compression correctness - Avoid per-copy `IoBufferCore` heap allocations in `CompositeBuffer::CopyTo / ViewOrCopyRange` iterators; add fast path for `BufferHeader::Read` when the 64-byte header fits in the first plain-memory segment. - `BufferHeader`: add `IsHeaderValid()` gate covering `BlockSizeExponent` range, `BlockCount * BlockSize` overflow, and `TotalRawSize` bounds before any arithmetic uses them. Defends against attacker-controlled headers that can pass the CRC and trigger OOB writes in `DecompressBlock`.
Diffstat (limited to 'src/zenutil/process/exitwatcher.cpp')
-rw-r--r--src/zenutil/process/exitwatcher.cpp61
1 files changed, 41 insertions, 20 deletions
diff --git a/src/zenutil/process/exitwatcher.cpp b/src/zenutil/process/exitwatcher.cpp
index cef31ebca..2e88dfdeb 100644
--- a/src/zenutil/process/exitwatcher.cpp
+++ b/src/zenutil/process/exitwatcher.cpp
@@ -38,7 +38,7 @@ namespace zen {
#if ZEN_PLATFORM_LINUX
-struct ProcessExitWatcher::Impl
+struct ProcessExitWatcher::Impl : public TRefCounted<Impl>
{
asio::io_context& m_IoContext;
std::unique_ptr<asio::posix::stream_descriptor> m_Descriptor;
@@ -64,8 +64,11 @@ struct ProcessExitWatcher::Impl
m_Descriptor = std::make_unique<asio::posix::stream_descriptor>(m_IoContext, m_PidFd);
+ // Capture a strong Ref so the Impl outlives the pending async_wait even
+ // if the owning ProcessExitWatcher is destroyed while a completion is
+ // in flight on the io_context.
m_Descriptor->async_wait(asio::posix::stream_descriptor::wait_read,
- [this, Callback = std::move(OnExit)](const asio::error_code& Ec) {
+ [Self = Ref<Impl>(this), Callback = std::move(OnExit)](const asio::error_code& Ec) {
if (Ec)
{
return; // Cancelled or error
@@ -74,7 +77,7 @@ struct ProcessExitWatcher::Impl
int ExitCode = -1;
int Status = 0;
// The pidfd told us the process exited. Reap it with waitpid.
- if (waitpid(m_Pid, &Status, WNOHANG) > 0)
+ if (waitpid(Self->m_Pid, &Status, WNOHANG) > 0)
{
if (WIFEXITED(Status))
{
@@ -115,7 +118,7 @@ struct ProcessExitWatcher::Impl
#elif ZEN_PLATFORM_WINDOWS
-struct ProcessExitWatcher::Impl
+struct ProcessExitWatcher::Impl : public TRefCounted<Impl>
{
asio::io_context& m_IoContext;
std::unique_ptr<asio::windows::object_handle> m_ObjectHandle;
@@ -147,16 +150,21 @@ struct ProcessExitWatcher::Impl
// object_handle takes ownership of the handle
m_ObjectHandle = std::make_unique<asio::windows::object_handle>(m_IoContext, m_DuplicatedHandle);
- m_ObjectHandle->async_wait([this, DupHandle = m_DuplicatedHandle, Callback = std::move(OnExit)](const asio::error_code& Ec) {
- if (Ec)
- {
- return;
- }
-
- DWORD ExitCode = 0;
- GetExitCodeProcess(static_cast<HANDLE>(DupHandle), &ExitCode);
- Callback(static_cast<int>(ExitCode));
- });
+ // Capture a strong Ref so the duplicated handle (owned by m_ObjectHandle,
+ // which is owned by *this) cannot be closed out from under a completion
+ // that the io_context is still about to dispatch.
+ m_ObjectHandle->async_wait(
+ [Self = Ref<Impl>(this), DupHandle = m_DuplicatedHandle, Callback = std::move(OnExit)](const asio::error_code& Ec) {
+ (void)Self;
+ if (Ec)
+ {
+ return;
+ }
+
+ DWORD ExitCode = 0;
+ GetExitCodeProcess(static_cast<HANDLE>(DupHandle), &ExitCode);
+ Callback(static_cast<int>(ExitCode));
+ });
}
void Cancel()
@@ -182,7 +190,7 @@ struct ProcessExitWatcher::Impl
#elif ZEN_PLATFORM_MAC
-struct ProcessExitWatcher::Impl
+struct ProcessExitWatcher::Impl : public TRefCounted<Impl>
{
asio::io_context& m_IoContext;
std::unique_ptr<asio::posix::stream_descriptor> m_Descriptor;
@@ -218,8 +226,11 @@ struct ProcessExitWatcher::Impl
m_Descriptor = std::make_unique<asio::posix::stream_descriptor>(m_IoContext, m_KqueueFd);
+ // Capture a strong Ref so the Impl outlives the pending async_wait even
+ // if the owning ProcessExitWatcher is destroyed while a completion is
+ // in flight on the io_context.
m_Descriptor->async_wait(asio::posix::stream_descriptor::wait_read,
- [this, Callback = std::move(OnExit)](const asio::error_code& Ec) {
+ [Self = Ref<Impl>(this), Callback = std::move(OnExit)](const asio::error_code& Ec) {
if (Ec)
{
return;
@@ -228,11 +239,11 @@ struct ProcessExitWatcher::Impl
// Drain the kqueue event
struct kevent Event;
struct timespec Timeout = {0, 0};
- kevent(m_KqueueFd, nullptr, 0, &Event, 1, &Timeout);
+ kevent(Self->m_KqueueFd, nullptr, 0, &Event, 1, &Timeout);
int ExitCode = -1;
int Status = 0;
- if (waitpid(m_Pid, &Status, WNOHANG) > 0)
+ if (waitpid(Self->m_Pid, &Status, WNOHANG) > 0)
{
if (WIFEXITED(Status))
{
@@ -273,11 +284,21 @@ struct ProcessExitWatcher::Impl
// Common wrapper (delegates to Impl)
// ============================================================================
-ProcessExitWatcher::ProcessExitWatcher(asio::io_context& IoContext) : m_Impl(std::make_unique<Impl>(IoContext))
+ProcessExitWatcher::ProcessExitWatcher(asio::io_context& IoContext) : m_Impl(new Impl(IoContext))
{
}
-ProcessExitWatcher::~ProcessExitWatcher() = default;
+ProcessExitWatcher::~ProcessExitWatcher()
+{
+ // Explicitly cancel pending async ops. The Impl may outlive this call if a
+ // completion is still in the io_context queue (the handler holds a strong
+ // Ref back to Impl to keep it alive). Cancel() here guarantees the watch
+ // stops even if nobody has called Cancel() on the wrapper.
+ if (m_Impl)
+ {
+ m_Impl->Cancel();
+ }
+}
void
ProcessExitWatcher::Watch(const ProcessHandle& Handle, std::function<void(int ExitCode)> OnExit)