aboutsummaryrefslogtreecommitdiff
path: root/src/zenserver/storage/objectstore/objectstore.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/zenserver/storage/objectstore/objectstore.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/zenserver/storage/objectstore/objectstore.cpp')
-rw-r--r--src/zenserver/storage/objectstore/objectstore.cpp61
1 files changed, 41 insertions, 20 deletions
diff --git a/src/zenserver/storage/objectstore/objectstore.cpp b/src/zenserver/storage/objectstore/objectstore.cpp
index 1115c1cd6..252a381ae 100644
--- a/src/zenserver/storage/objectstore/objectstore.cpp
+++ b/src/zenserver/storage/objectstore/objectstore.cpp
@@ -29,6 +29,15 @@ using namespace std::literals;
ZEN_DEFINE_LOG_CATEGORY_STATIC(LogObj, "obj"sv);
+namespace {
+ // Permitted bucket-name characters. Must stay in sync with the "bucket" URL matcher
+ // registered in HttpObjectStoreService::Initialize so POST / DELETE / PUT with an
+ // explicit bucketname payload uses the same rule as the route matcher.
+ constexpr AsciiSet ValidBucketCharactersSet{"abcdefghijklmnopqrstuvwxyz0123456789-_.ABCDEFGHIJKLMNOPQRSTUVWXYZ"};
+
+ bool IsValidBucketName(std::string_view Name) { return !Name.empty() && AsciiSet::HasOnly(Name, ValidBucketCharactersSet); }
+} // namespace
+
class CbXmlWriter
{
public:
@@ -302,12 +311,10 @@ HttpObjectStoreService::Initialize()
}
static constexpr AsciiSet ValidPathCharactersSet{"abcdefghijklmnopqrstuvwxyz0123456789/_.,;$~{}+-[]() ABCDEFGHIJKLMNOPQRSTUVWXYZ"};
- static constexpr AsciiSet ValidBucketCharactersSet{"abcdefghijklmnopqrstuvwxyz0123456789-_.ABCDEFGHIJKLMNOPQRSTUVWXYZ"};
m_Router.AddMatcher("path",
[](std::string_view Str) -> bool { return !Str.empty() && AsciiSet::HasOnly(Str, ValidPathCharactersSet); });
- m_Router.AddMatcher("bucket",
- [](std::string_view Str) -> bool { return !Str.empty() && AsciiSet::HasOnly(Str, ValidBucketCharactersSet); });
+ m_Router.AddMatcher("bucket", [](std::string_view Str) -> bool { return IsValidBucketName(Str); });
m_Router.RegisterRoute(
"",
@@ -471,8 +478,9 @@ HttpObjectStoreService::CreateBucket(HttpRouterRequest& Request)
const CbObject Params = Request.ServerRequest().ReadPayloadObject();
const std::string_view BucketName = Params["bucketname"].AsString();
- if (BucketName.empty())
+ if (!IsValidBucketName(BucketName))
{
+ ZEN_LOG_WARN(LogObj, "CREATE - rejected invalid bucket name '{}'", BucketName);
return Request.ServerRequest().WriteResponse(HttpResponseCode::BadRequest);
}
@@ -515,9 +523,21 @@ HttpObjectStoreService::ListBucket(HttpRouterRequest& Request, const std::string
BucketPrefix.erase(0, BucketPrefix.find_first_not_of('/'));
BucketPrefix.erase(0, BucketPrefix.find_first_not_of('\\'));
- const fs::path BucketRoot = GetBucketDirectory(BucketName);
- const fs::path RelativeBucketPath = fs::path(BucketPrefix).make_preferred();
- const fs::path FullPath = BucketRoot / RelativeBucketPath;
+ const fs::path BucketRoot = GetBucketDirectory(BucketName);
+
+ fs::path RelativeBucketPath;
+ fs::path FullPath = BucketRoot;
+ if (!BucketPrefix.empty())
+ {
+ std::optional<fs::path> Resolved = ResolveSafeRelativePath(BucketRoot, BucketPrefix);
+ if (!Resolved)
+ {
+ ZEN_LOG_WARN(LogObj, "LIST - bucket '{}' rejected unsafe prefix '{}'", BucketName, BucketPrefix);
+ return Request.ServerRequest().WriteResponse(HttpResponseCode::Forbidden);
+ }
+ FullPath = std::move(*Resolved);
+ RelativeBucketPath = fs::relative(FullPath, BucketRoot);
+ }
struct Visitor : FileSystemTraversal::TreeVisitor
{
@@ -589,8 +609,9 @@ HttpObjectStoreService::DeleteBucket(HttpRouterRequest& Request)
const CbObject Params = Request.ServerRequest().ReadPayloadObject();
const std::string_view BucketName = Params["bucketname"].AsString();
- if (BucketName.empty())
+ if (!IsValidBucketName(BucketName))
{
+ ZEN_LOG_WARN(LogObj, "DELETE - rejected invalid bucket name '{}'", BucketName);
return Request.ServerRequest().WriteResponse(HttpResponseCode::BadRequest);
}
@@ -621,15 +642,14 @@ HttpObjectStoreService::GetObject(HttpRouterRequest& Request, const std::string_
return Request.ServerRequest().WriteResponse(HttpResponseCode::NotFound);
}
- const fs::path RelativeBucketPath = fs::path(BucketPrefix).make_preferred();
-
- if (RelativeBucketPath.is_absolute() || RelativeBucketPath.string().starts_with(".."))
+ std::optional<fs::path> ResolvedFilePath = ResolveSafeRelativePath(BucketDir, BucketPrefix);
+ if (!ResolvedFilePath)
{
- ZEN_LOG_DEBUG(LogObj, "GET - from bucket '{}' [FAILED], invalid file path", BucketName);
+ ZEN_LOG_WARN(LogObj, "GET - from bucket '{}' rejected unsafe path '{}'", BucketName, BucketPrefix);
return Request.ServerRequest().WriteResponse(HttpResponseCode::Forbidden);
}
-
- const fs::path FilePath = BucketDir / RelativeBucketPath;
+ const fs::path FilePath = std::move(*ResolvedFilePath);
+ const fs::path RelativeBucketPath = fs::relative(FilePath, BucketDir);
if (!IsFile(FilePath))
{
ZEN_LOG_DEBUG(LogObj, "GET - '{}/{}' [FAILED], doesn't exist", BucketName, FilePath);
@@ -720,16 +740,17 @@ HttpObjectStoreService::PutObject(HttpRouterRequest& Request)
return Request.ServerRequest().WriteResponse(HttpResponseCode::NotFound);
}
- const fs::path RelativeBucketPath = fs::path(Request.GetCapture(2)).make_preferred();
+ const std::string_view PathCapture = Request.GetCapture(2);
- if (RelativeBucketPath.is_absolute() || RelativeBucketPath.string().starts_with(".."))
+ std::optional<fs::path> ResolvedFilePath = ResolveSafeRelativePath(BucketDir, PathCapture);
+ if (!ResolvedFilePath)
{
- ZEN_LOG_DEBUG(LogObj, "PUT - bucket '{}' [FAILED], invalid file path", BucketName);
+ ZEN_LOG_WARN(LogObj, "PUT - bucket '{}' rejected unsafe path '{}'", BucketName, PathCapture);
return Request.ServerRequest().WriteResponse(HttpResponseCode::Forbidden);
}
-
- const fs::path FilePath = BucketDir / RelativeBucketPath;
- const fs::path FileDirectory = FilePath.parent_path();
+ const fs::path FilePath = std::move(*ResolvedFilePath);
+ const fs::path RelativeBucketPath = fs::relative(FilePath, BucketDir);
+ const fs::path FileDirectory = FilePath.parent_path();
{
std::lock_guard _(m_BucketsMutex);