aboutsummaryrefslogtreecommitdiff
path: root/src/zencore/basicfile.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/basicfile.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/basicfile.cpp')
-rw-r--r--src/zencore/basicfile.cpp65
1 files changed, 60 insertions, 5 deletions
diff --git a/src/zencore/basicfile.cpp b/src/zencore/basicfile.cpp
index fdf742261..01d550957 100644
--- a/src/zencore/basicfile.cpp
+++ b/src/zencore/basicfile.cpp
@@ -798,11 +798,12 @@ BasicFileWriter::Write(const void* Data, uint64_t Size, uint64_t FileOffset)
{
if (m_Buffer == nullptr || (Size >= m_BufferSize))
{
- if (FileOffset == m_BufferEnd)
- {
- Flush();
- m_BufferStart = m_BufferEnd = FileOffset + Size;
- }
+ // Always flush pending buffered data first. Otherwise a later
+ // Flush() would replay stale bytes at m_BufferStart, clobbering
+ // any range of this direct write that overlaps
+ // [m_BufferStart, m_BufferEnd).
+ Flush();
+ m_BufferStart = m_BufferEnd = FileOffset + Size;
m_Base.Write(Data, Size, FileOffset);
return;
@@ -1200,6 +1201,60 @@ TEST_CASE("BasicFileBuffer")
}
}
+TEST_CASE("BasicFileWriter.LargeDiscontinuousWriteFlushesBuffer")
+{
+ // Regression: BasicFileWriter::Write's large-write branch used to skip
+ // Flush() whenever FileOffset != m_BufferEnd. Any subsequent Flush
+ // (including the one in ~BasicFileWriter) then replayed the stale
+ // buffered bytes at their original offset, clobbering whatever the
+ // caller had just written directly.
+ ScopedCurrentDirectoryChange _;
+
+ constexpr uint64_t BufferSize = 64;
+ constexpr uint64_t SmallSize = 10;
+ constexpr uint64_t LargeSize = 1024; // >= BufferSize, forces the direct-write path
+ constexpr uint64_t LargeOffset = 5; // overlaps the pending small-write region
+ constexpr uint64_t OverlapStart = LargeOffset;
+ constexpr uint64_t OverlapEnd = SmallSize;
+
+ {
+ BasicFile File;
+ File.Open("discontig_write", BasicFile::Mode::kTruncate);
+ BasicFileWriter Writer(File, BufferSize);
+
+ // First: small write buffered at [0, SmallSize) - still pending flush.
+ std::vector<uint8_t> Small(SmallSize, 'A');
+ Writer.Write(Small.data(), Small.size(), 0);
+
+ // Second: large write that overlaps the pending buffered region.
+ // Last-writer-wins means the overlap bytes must end up as 'B'.
+ std::vector<uint8_t> Large(LargeSize, 'B');
+ Writer.Write(Large.data(), Large.size(), LargeOffset);
+ }
+
+ BasicFile Reader;
+ Reader.Open("discontig_write", BasicFile::Mode::kRead);
+ IoBuffer Contents = Reader.ReadAll();
+ REQUIRE_EQ(Contents.Size(), LargeOffset + LargeSize);
+
+ const uint8_t* Bytes = reinterpret_cast<const uint8_t*>(Contents.Data());
+ for (uint64_t I = 0; I < OverlapStart; ++I)
+ {
+ CHECK_EQ(Bytes[I], 'A');
+ }
+ // The bytes in the overlap range [OverlapStart, OverlapEnd) are the
+ // critical check: with the bug, the late Flush() replayed the small
+ // 'A' write and clobbered these with 'A'.
+ for (uint64_t I = OverlapStart; I < OverlapEnd; ++I)
+ {
+ CHECK_EQ(Bytes[I], 'B');
+ }
+ for (uint64_t I = OverlapEnd; I < LargeOffset + LargeSize; ++I)
+ {
+ CHECK_EQ(Bytes[I], 'B');
+ }
+}
+
TEST_SUITE_END();
void