aboutsummaryrefslogtreecommitdiff
path: root/src/zencore/compress.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/zencore/compress.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/zencore/compress.cpp')
-rw-r--r--src/zencore/compress.cpp81
1 files changed, 73 insertions, 8 deletions
diff --git a/src/zencore/compress.cpp b/src/zencore/compress.cpp
index 6aa0adce0..a0e91f908 100644
--- a/src/zencore/compress.cpp
+++ b/src/zencore/compress.cpp
@@ -78,12 +78,20 @@ struct BufferHeader
BufferHeader Header;
if (sizeof(BufferHeader) <= CompressedData.GetSize())
{
- // if (CompressedData.GetSegments()[0].AsIoBuffer().IsWholeFile())
- // {
- // ZEN_ASSERT(true);
- // }
- CompositeBuffer::Iterator It;
- CompressedData.CopyTo(MakeMutableMemoryView(&Header, &Header + 1), It);
+ // Fast path: the overwhelmingly common case is that the 64-byte header sits entirely
+ // within the first segment and that segment is plain memory. Skip the iterator and
+ // the sub-range IoBuffer wrapper (which would otherwise heap-allocate an IoBufferCore).
+ const std::span<const SharedBuffer> Segments = CompressedData.GetSegments();
+ const SharedBuffer& First = Segments.front();
+ if (sizeof(BufferHeader) <= First.GetSize() && !First.IsExtended())
+ {
+ MakeMutableMemoryView(&Header, &Header + 1).CopyFrom(First.GetView().Left(sizeof(BufferHeader)));
+ }
+ else
+ {
+ CompositeBuffer::Iterator It;
+ CompressedData.CopyTo(MakeMutableMemoryView(&Header, &Header + 1), It);
+ }
Header.ByteSwap();
}
return Header;
@@ -837,6 +845,15 @@ BlockDecoder::DecompressToStream(
{
return false;
}
+ // RawOffset+RawSize-1 below underflows when RawSize is 0, and the
+ // BlockCount-0 / BlockSize-0 arithmetic is only defined when the header
+ // has already been validated (see IsHeaderValid). Guard both here as
+ // defence in depth.
+ if (RawSize == 0 || Header.BlockCount == 0 || Header.BlockSizeExponent >= 32 || RawOffset > Header.TotalRawSize ||
+ RawSize > Header.TotalRawSize - RawOffset)
+ {
+ return false;
+ }
const uint64_t BlockSize = uint64_t(1) << Header.BlockSizeExponent;
@@ -1386,6 +1403,50 @@ GetDecoder(CompressionMethod Method)
}
}
+// Sanity-check a header that was just read from an untrusted buffer before
+// any of the decode arithmetic (1 << BlockSizeExponent, BlockCount*BlockSize,
+// divides by BlockSize, etc.) is performed. Must be called after the magic,
+// decoder and CRC checks pass.
+static bool
+IsHeaderValid(const BufferHeader& Header)
+{
+ // 1 << BlockSizeExponent is UB for Exponent >= 64 and wildly impractical
+ // below that. Real producers use <= 24 (16 MiB blocks); cap at 32 for
+ // headroom while staying well below the UB boundary.
+ if (Header.BlockSizeExponent >= 32)
+ {
+ return false;
+ }
+
+ // Only the block-based methods use BlockCount / BlockSizeExponent. The
+ // None method keeps them zero.
+ if (Header.Method != CompressionMethod::None)
+ {
+ // A non-empty buffer needs at least one block.
+ if (Header.BlockCount == 0)
+ {
+ return Header.TotalRawSize == 0;
+ }
+
+ const uint64_t BlockSize = uint64_t(1) << Header.BlockSizeExponent;
+
+ // BlockCount * BlockSize must not overflow and must fit TotalRawSize
+ // in the half-open range ((BlockCount - 1) * BlockSize, BlockCount * BlockSize].
+ if (Header.BlockCount > (UINT64_MAX / BlockSize))
+ {
+ return false;
+ }
+ const uint64_t MaxRawSize = uint64_t(Header.BlockCount) * BlockSize;
+ const uint64_t MinRawSize = MaxRawSize - BlockSize;
+ if (Header.TotalRawSize > MaxRawSize || Header.TotalRawSize <= MinRawSize)
+ {
+ return false;
+ }
+ }
+
+ return true;
+}
+
//////////////////////////////////////////////////////////////////////////
bool
@@ -1426,6 +1487,10 @@ ReadHeader(const CompositeBuffer& CompressedData, BufferHeader& OutHeader, Uniqu
{
return false;
}
+ if (!IsHeaderValid(OutHeader))
+ {
+ return false;
+ }
uint64_t FullHeaderSize = Decoder->GetHeaderSize(OutHeader);
if (FullHeaderSize > CompressedDataSize)
{
@@ -1520,7 +1585,7 @@ TryReadHeader(DecoderContext& Context, Archive& Ar, FHeader& OutHeader, MemoryVi
FHeader* const HeaderCopy = static_cast<FHeader*>(HeaderView.GetData());
HeaderCopy->ByteSwap();
- if (Header.Crc32 == FHeader::CalculateCrc32(HeaderView))
+ if (Header.Crc32 == FHeader::CalculateCrc32(HeaderView) && IsHeaderValid(Header))
{
Context.HeaderOffset = uint64_t(Offset);
Context.HeaderSize = HeaderSize;
@@ -1560,7 +1625,7 @@ TryReadHeader(DecoderContext& Context, const CompositeBuffer& Buffer, FHeader& O
const MemoryView HeaderView = Buffer.ViewOrCopyRange(0, HeaderSize, Context.Header, [](uint64_t Size) {
return UniqueBuffer::Alloc(zen::Max(NextPow2(Size), DefaultHeaderSize));
});
- if (Header.Crc32 == FHeader::CalculateCrc32(HeaderView))
+ if (Header.Crc32 == FHeader::CalculateCrc32(HeaderView) && IsHeaderValid(Header))
{
Context.HeaderOffset = 0;
Context.HeaderSize = HeaderSize;