diff options
| author | Stefan Boberg <[email protected]> | 2026-05-04 16:46:03 +0200 |
|---|---|---|
| committer | GitHub Enterprise <[email protected]> | 2026-05-04 16:46:03 +0200 |
| commit | 10d2a61fe1c848f44033e8450ff3a5ffa7f4322a (patch) | |
| tree | aa66c6a068b50d2390bdae5f857c7151f15e5a86 /src/zenserver/sessions/httpsessions.cpp | |
| parent | Tui picker fixes (#1027) (diff) | |
| download | archived-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/zenserver/sessions/httpsessions.cpp')
| -rw-r--r-- | src/zenserver/sessions/httpsessions.cpp | 26 |
1 files changed, 22 insertions, 4 deletions
diff --git a/src/zenserver/sessions/httpsessions.cpp b/src/zenserver/sessions/httpsessions.cpp index 2276cb81a..88db36828 100644 --- a/src/zenserver/sessions/httpsessions.cpp +++ b/src/zenserver/sessions/httpsessions.cpp @@ -5,6 +5,7 @@ #include <zencore/compactbinarybuilder.h> #include <zencore/fmtutils.h> #include <zencore/logging.h> +#include <zencore/string.h> #include <zencore/trace.h> #include "sessions.h" @@ -470,9 +471,14 @@ HttpSessionsService::SessionLogRequest(HttpRouterRequest& Req) std::string_view CursorStr = Params.GetValue("cursor"sv); if (!CursorStr.empty()) { - uint64_t AfterCursor = std::strtoull(std::string(CursorStr).c_str(), nullptr, 10); + const std::optional<uint64_t> AfterCursor = ParseInt<uint64_t>(CursorStr); + if (!AfterCursor) + { + m_SessionsStats.BadRequestCount++; + return ServerRequest.WriteResponse(HttpResponseCode::BadRequest, HttpContentType::kText, "Invalid 'cursor' parameter"sv); + } - SessionsService::Session::CursorResult Result = Session->GetLogEntriesAfter(AfterCursor); + SessionsService::Session::CursorResult Result = Session->GetLogEntriesAfter(*AfterCursor); CbObjectWriter Response; Response << "cursor" << Result.Cursor; @@ -495,11 +501,23 @@ HttpSessionsService::SessionLogRequest(HttpRouterRequest& Req) if (std::string_view LimitStr = Params.GetValue("limit"sv); !LimitStr.empty()) { - Limit = uint32_t(std::strtoul(std::string(LimitStr).c_str(), nullptr, 10)); + const std::optional<uint32_t> Parsed = ParseInt<uint32_t>(LimitStr); + if (!Parsed) + { + m_SessionsStats.BadRequestCount++; + return ServerRequest.WriteResponse(HttpResponseCode::BadRequest, HttpContentType::kText, "Invalid 'limit' parameter"sv); + } + Limit = *Parsed; } if (std::string_view OffsetStr = Params.GetValue("offset"sv); !OffsetStr.empty()) { - Offset = uint32_t(std::strtoul(std::string(OffsetStr).c_str(), nullptr, 10)); + const std::optional<uint32_t> Parsed = ParseInt<uint32_t>(OffsetStr); + if (!Parsed) + { + m_SessionsStats.BadRequestCount++; + return ServerRequest.WriteResponse(HttpResponseCode::BadRequest, HttpContentType::kText, "Invalid 'offset' parameter"sv); + } + Offset = *Parsed; } std::vector<SessionsService::LogEntry> Entries = Session->GetLogEntries(Limit, Offset); |