add validation of compact binary payloads before reading them (#483)

* add validation of compact binary payloads before reading them

7 files changed, 266 insertions, 199 deletions

diff --git a/src/zenserver/cache/httpstructuredcache.cpp b/src/zenserver/cache/httpstructuredcache.cpp
index 68f1c602e..08d0b12a7 100644
--- a/src/zenserver/cache/httpstructuredcache.cpp
+++ b/src/zenserver/cache/httpstructuredcache.cpp
@@ -5,6 +5,7 @@
 #include <zencore/compactbinary.h>
 #include <zencore/compactbinarybuilder.h>
 #include <zencore/compactbinarypackage.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compactbinaryvalidation.h>
 #include <zencore/compress.h>
 #include <zencore/enumflags.h>
@@ -842,45 +843,61 @@ HttpStructuredCacheService::HandleGetCacheRecord(HttpServerRequest& Request, con
 		{
 			if (ContentType == ZenContentType::kCbObject)
 			{
-				CbPackage Package;
-				uint32_t  MissingCount = 0;
-
-				CbObjectView CacheRecord(ClientResultValue.Value.Data());
-				CacheRecord.IterateAttachments([this, &MissingCount, &Package, SkipData](CbFieldView AttachmentHash) {
-					if (SkipData)
-					{
-						if (!m_CidStore.ContainsChunk(AttachmentHash.AsHash()))
+				CbPackage		Package;
+				uint32_t		MissingCount  = 0;
+				CbValidateError ValidateError = CbValidateError::None;
+				if (CbObject PackageObject = ValidateAndReadCompactBinaryObject(std::move(ClientResultValue.Value), ValidateError);
+					ValidateError == CbValidateError::None)
+				{
+					CbObjectView CacheRecord(ClientResultValue.Value.Data());
+					CacheRecord.IterateAttachments([this, &MissingCount, &Package, SkipData](CbFieldView AttachmentHash) {
+						if (SkipData)
 						{
-							MissingCount++;
+							if (!m_CidStore.ContainsChunk(AttachmentHash.AsHash()))
+							{
+								MissingCount++;
+							}
 						}
-					}
-					else
-					{
-						if (IoBuffer Chunk = m_CidStore.FindChunkByCid(AttachmentHash.AsHash()))
+						else
 						{
-							CompressedBuffer Compressed = CompressedBuffer::FromCompressedNoValidate(std::move(Chunk));
-							if (Compressed)
+							if (IoBuffer Chunk = m_CidStore.FindChunkByCid(AttachmentHash.AsHash()))
 							{
-								Package.AddAttachment(CbAttachment(Compressed, AttachmentHash.AsHash()));
+								CompressedBuffer Compressed = CompressedBuffer::FromCompressedNoValidate(std::move(Chunk));
+								if (Compressed)
+								{
+									Package.AddAttachment(CbAttachment(Compressed, AttachmentHash.AsHash()));
+								}
+								else
+								{
+									ZEN_WARN("invalid compressed binary returned for {}", AttachmentHash.AsHash());
+									MissingCount++;
+								}
 							}
 							else
 							{
-								ZEN_WARN("invalid compressed binary returned for {}", AttachmentHash.AsHash());
 								MissingCount++;
 							}
 						}
-						else
-						{
-							MissingCount++;
-						}
-					}
-				});
+					});
 
-				Success = MissingCount == 0 || PartialRecord;
+					Success = MissingCount == 0 || PartialRecord;
+				}
+				else
+				{
+					ZEN_WARN("Invalid compact binary payload returned for {}/{}/{} ({}). Reason: '{}'",
+							 Ref.Namespace,
+							 Ref.BucketSegment,
+							 Ref.HashKey,
+							 Ref.ValueContentId,
+							 ToString(ValidateError));
+					Success = false;
+				}
 
 				if (Success)
 				{
-					Package.SetObject(LoadCompactBinaryObject(ClientResultValue.Value));
+					CbObject PackageObject = LoadCompactBinaryObject(std::move(ClientResultValue.Value));
+
+					Package.SetObject(std::move(PackageObject));
 
 					BinaryWriter MemStream;
 					Package.Save(MemStream);
diff --git a/src/zenserver/config.cpp b/src/zenserver/config.cpp
index fb2d9b7f4..0397677b9 100644
--- a/src/zenserver/config.cpp
+++ b/src/zenserver/config.cpp
@@ -7,6 +7,7 @@
 
 #include <zencore/basicfile.h>
 #include <zencore/compactbinarybuilder.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compactbinaryvalidation.h>
 #include <zencore/crypto.h>
 #include <zencore/except.h>
@@ -72,13 +73,12 @@ ReadAllCentralManifests(const std::filesystem::path& SystemRoot)
 	{
 		try
 		{
-			FileContents	FileData	  = ReadFile(File);
-			IoBuffer		DataBuffer	  = FileData.Flatten();
-			CbValidateError ValidateError = ValidateCompactBinary(DataBuffer, CbValidateMode::All);
-
-			if (ValidateError == CbValidateError::None)
+			FileContents	FileData = ReadFile(File);
+			CbValidateError ValidateError;
+			if (CbObject Manifest = ValidateAndReadCompactBinaryObject(FileData.Flatten(), ValidateError);
+				ValidateError == CbValidateError::None)
 			{
-				Manifests.push_back(LoadCompactBinaryObject(DataBuffer));
+				Manifests.emplace_back(std::move(Manifest));
 			}
 			else
 			{
diff --git a/src/zenserver/projectstore/fileremoteprojectstore.cpp b/src/zenserver/projectstore/fileremoteprojectstore.cpp
index 375e44e59..e550222fd 100644
--- a/src/zenserver/projectstore/fileremoteprojectstore.cpp
+++ b/src/zenserver/projectstore/fileremoteprojectstore.cpp
@@ -2,6 +2,7 @@
 
 #include "fileremoteprojectstore.h"
 
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compress.h>
 #include <zencore/filesystem.h>
 #include <zencore/fmtutils.h>
@@ -260,11 +261,14 @@ private:
 			ContainerPayload = ContainerFile.ReadAll();
 		}
 		AddStats(0, ContainerPayload.GetSize(), Timer.GetElapsedTimeUs() * 1000);
-		Result.ContainerObject = LoadCompactBinaryObject(ContainerPayload);
-		if (!Result.ContainerObject)
+		CbValidateError ValidateResult = CbValidateError::None;
+		if (Result.ContainerObject = ValidateAndReadCompactBinaryObject(std::move(ContainerPayload), ValidateResult);
+			ValidateResult != CbValidateError::None || !Result.ContainerObject)
 		{
 			Result.ErrorCode	  = gsl::narrow<int32_t>(HttpResponseCode::InternalServerError);
-			Result.Reason		  = fmt::format("The file {} is not formatted as a compact binary object", SourcePath.string());
+			Result.Reason		  = fmt::format("The file {} is not formatted as a compact binary object ('{}')",
+										SourcePath.string(),
+										ToString(ValidateResult));
 			Result.ElapsedSeconds = Timer.GetElapsedTimeMs() / 1000.0;
 			return Result;
 		}
diff --git a/src/zenserver/projectstore/httpprojectstore.cpp b/src/zenserver/projectstore/httpprojectstore.cpp
index feeec3e37..237dc097e 100644
--- a/src/zenserver/projectstore/httpprojectstore.cpp
+++ b/src/zenserver/projectstore/httpprojectstore.cpp
@@ -1050,36 +1050,44 @@ HttpProjectService::HandleOplogOpPrepRequest(HttpRouterRequest& Req)
 	// chunks are not present on this server. This list is then returned in
 	// the "need" list in the response
 
-	IoBuffer Payload	   = HttpReq.ReadPayload();
-	CbObject RequestObject = LoadCompactBinaryObject(Payload);
+	CbValidateError ValidateResult;
+	if (CbObject RequestObject = ValidateAndReadCompactBinaryObject(HttpReq.ReadPayload(), ValidateResult);
+		ValidateResult == CbValidateError::None)
+	{
+		std::vector<IoHash> NeedList;
 
-	std::vector<IoHash> NeedList;
+		{
+			eastl::fixed_vector<IoHash, 16> ChunkList;
+			CbArrayView						HaveList = RequestObject["have"sv].AsArrayView();
+			ChunkList.reserve(HaveList.Num());
+			for (auto& Entry : HaveList)
+			{
+				ChunkList.push_back(Entry.AsHash());
+			}
 
-	{
-		eastl::fixed_vector<IoHash, 16> ChunkList;
-		CbArrayView						HaveList = RequestObject["have"sv].AsArrayView();
-		ChunkList.reserve(HaveList.Num());
-		for (auto& Entry : HaveList)
+			NeedList = FoundLog->CheckPendingChunkReferences(std::span(begin(ChunkList), end(ChunkList)), std::chrono::minutes(2));
+		}
+
+		CbObjectWriter Cbo(1 + 1 + 5 + NeedList.size() * (1 + sizeof(IoHash::Hash)) + 1);
+		Cbo.BeginArray("need");
 		{
-			ChunkList.push_back(Entry.AsHash());
+			for (const IoHash& Hash : NeedList)
+			{
+				ZEN_DEBUG("prep - NEED: {}", Hash);
+				Cbo << Hash;
+			}
 		}
+		Cbo.EndArray();
+		CbObject Response = Cbo.Save();
 
-		NeedList = FoundLog->CheckPendingChunkReferences(std::span(begin(ChunkList), end(ChunkList)), std::chrono::minutes(2));
+		return HttpReq.WriteResponse(HttpResponseCode::OK, Response);
 	}
-
-	CbObjectWriter Cbo(1 + 1 + 5 + NeedList.size() * (1 + sizeof(IoHash::Hash)) + 1);
-	Cbo.BeginArray("need");
+	else
 	{
-		for (const IoHash& Hash : NeedList)
-		{
-			ZEN_DEBUG("prep - NEED: {}", Hash);
-			Cbo << Hash;
-		}
+		return HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+									 HttpContentType::kText,
+									 fmt::format("Invalid compact binary format: '{}'", ToString(ValidateResult)));
 	}
-	Cbo.EndArray();
-	CbObject Response = Cbo.Save();
-
-	return HttpReq.WriteResponse(HttpResponseCode::OK, Response);
 }
 
 void
@@ -1173,7 +1181,9 @@ HttpProjectService::HandleOplogOpNewRequest(HttpRouterRequest& Req)
 
 	if (!legacy::TryLoadCbPackage(Package, Payload, &UniqueBuffer::Alloc, &Resolver))
 	{
-		if (CbObject Core = LoadCompactBinaryObject(Payload))
+		CbValidateError ValidateResult;
+		if (CbObject Core = ValidateAndReadCompactBinaryObject(IoBuffer(Payload), ValidateResult);
+			ValidateResult == CbValidateError::None && Core)
 		{
 			Package.SetObject(Core);
 		}
@@ -1182,7 +1192,7 @@ HttpProjectService::HandleOplogOpNewRequest(HttpRouterRequest& Req)
 			std::filesystem::path BadPackagePath =
 				Oplog.TempPath() / "bad_packages"sv / fmt::format("session{}_request{}"sv, HttpReq.SessionId(), HttpReq.RequestId());
 
-			ZEN_WARN("Received malformed package! Saving payload to '{}'", BadPackagePath);
+			ZEN_WARN("Received malformed package ('{}')! Saving payload to '{}'", ToString(ValidateResult), BadPackagePath);
 
 			WriteFile(BadPackagePath, Payload);
 
@@ -1413,15 +1423,18 @@ HttpProjectService::HandleOpLogOpRequest(HttpRouterRequest& Req)
 						switch (Payload.GetContentType())
 						{
 							case ZenContentType::kCbObject:
-								if (CbObject Object = LoadCompactBinaryObject(Payload))
 								{
-									Package.AddAttachment(CbAttachment(Object));
-								}
-								else
-								{
-									// Error - malformed object
-
-									ZEN_WARN("malformed object returned for {}", AttachmentHash);
+									CbValidateError ValidateResult;
+									if (CbObject Object = ValidateAndReadCompactBinaryObject(std::move(Payload), ValidateResult);
+										ValidateResult == CbValidateError::None && Object)
+									{
+										Package.AddAttachment(CbAttachment(Object));
+									}
+									else
+									{
+										// Error - malformed object
+										ZEN_WARN("malformed object returned for {} ('{}')", AttachmentHash, ToString(ValidateResult));
+									}
 								}
 								break;
 
@@ -1774,66 +1787,20 @@ HttpProjectService::HandleProjectRequest(HttpRouterRequest& Req)
 					return HttpReq.WriteResponse(HttpResponseCode::InsufficientStorage);
 				}
 
-				IoBuffer			  Payload	 = HttpReq.ReadPayload();
-				CbObject			  Params	 = LoadCompactBinaryObject(Payload);
-				std::filesystem::path Root		 = Params["root"sv].AsU8String();	 // Workspace root (i.e `D:/UE5/`)
-				std::filesystem::path EngineRoot = Params["engine"sv].AsU8String();	 // Engine root (i.e `D:/UE5/Engine`)
-				std::filesystem::path ProjectRoot =
-					Params["project"sv].AsU8String();  // Project root directory (i.e `D:/UE5/Samples/Games/Lyra`)
-				std::filesystem::path ProjectFilePath =
-					Params["projectfile"sv].AsU8String();  // Project file path (i.e `D:/UE5/Samples/Games/Lyra/Lyra.uproject`)
-
-				const std::filesystem::path BasePath = m_ProjectStore->BasePath() / ProjectId;
-				m_ProjectStore->NewProject(BasePath, ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath);
-
-				ZEN_INFO("established project (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
-						 ProjectId,
-						 Root,
-						 EngineRoot,
-						 ProjectRoot,
-						 ProjectFilePath,
-						 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
-
-				m_ProjectStats.ProjectWriteCount++;
-				HttpReq.WriteResponse(HttpResponseCode::Created);
-			}
-			break;
-
-		case HttpVerb::kPut:
-			{
-				if (!m_ProjectStore->AreDiskWritesAllowed())
+				CbValidateError ValidateResult;
+				if (CbObject Params = ValidateAndReadCompactBinaryObject(HttpReq.ReadPayload(), ValidateResult);
+					ValidateResult == CbValidateError::None)
 				{
-					return HttpReq.WriteResponse(HttpResponseCode::InsufficientStorage);
-				}
+					std::filesystem::path Root		 = Params["root"sv].AsU8String();	 // Workspace root (i.e `D:/UE5/`)
+					std::filesystem::path EngineRoot = Params["engine"sv].AsU8String();	 // Engine root (i.e `D:/UE5/Engine`)
+					std::filesystem::path ProjectRoot =
+						Params["project"sv].AsU8String();  // Project root directory (i.e `D:/UE5/Samples/Games/Lyra`)
+					std::filesystem::path ProjectFilePath =
+						Params["projectfile"sv].AsU8String();  // Project file path (i.e `D:/UE5/Samples/Games/Lyra/Lyra.uproject`)
 
-				IoBuffer			  Payload	 = HttpReq.ReadPayload();
-				CbObject			  Params	 = LoadCompactBinaryObject(Payload);
-				std::filesystem::path Root		 = Params["root"sv].AsU8String();	 // Workspace root (i.e `D:/UE5/`)
-				std::filesystem::path EngineRoot = Params["engine"sv].AsU8String();	 // Engine root (i.e `D:/UE5/Engine`)
-				std::filesystem::path ProjectRoot =
-					Params["project"sv].AsU8String();  // Project root directory (i.e `D:/UE5/Samples/Games/Lyra`)
-				std::filesystem::path ProjectFilePath =
-					Params["projectfile"sv].AsU8String();  // Project file path (i.e `D:/UE5/Samples/Games/Lyra/Lyra.uproject`)
-
-				if (m_ProjectStore->UpdateProject(ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath))
-				{
-					m_ProjectStats.ProjectWriteCount++;
-					ZEN_INFO("updated project (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
-							 ProjectId,
-							 Root,
-							 EngineRoot,
-							 ProjectRoot,
-							 ProjectFilePath,
-							 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
-
-					HttpReq.WriteResponse(HttpResponseCode::OK);
-				}
-				else
-				{
 					const std::filesystem::path BasePath = m_ProjectStore->BasePath() / ProjectId;
 					m_ProjectStore->NewProject(BasePath, ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath);
 
-					m_ProjectStats.ProjectWriteCount++;
 					ZEN_INFO("established project (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
 							 ProjectId,
 							 Root,
@@ -1842,8 +1809,71 @@ HttpProjectService::HandleProjectRequest(HttpRouterRequest& Req)
 							 ProjectFilePath,
 							 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
 
+					m_ProjectStats.ProjectWriteCount++;
 					HttpReq.WriteResponse(HttpResponseCode::Created);
 				}
+				else
+				{
+					HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+										  HttpContentType::kText,
+										  fmt::format("Malformed compact binary object: '{}'", ToString(ValidateResult)));
+				}
+			}
+			break;
+
+		case HttpVerb::kPut:
+			{
+				if (!m_ProjectStore->AreDiskWritesAllowed())
+				{
+					return HttpReq.WriteResponse(HttpResponseCode::InsufficientStorage);
+				}
+				CbValidateError ValidateResult;
+				if (CbObject Params = ValidateAndReadCompactBinaryObject(HttpReq.ReadPayload(), ValidateResult);
+					ValidateResult == CbValidateError::None)
+				{
+					std::filesystem::path Root		 = Params["root"sv].AsU8String();	 // Workspace root (i.e `D:/UE5/`)
+					std::filesystem::path EngineRoot = Params["engine"sv].AsU8String();	 // Engine root (i.e `D:/UE5/Engine`)
+					std::filesystem::path ProjectRoot =
+						Params["project"sv].AsU8String();  // Project root directory (i.e `D:/UE5/Samples/Games/Lyra`)
+					std::filesystem::path ProjectFilePath =
+						Params["projectfile"sv].AsU8String();  // Project file path (i.e `D:/UE5/Samples/Games/Lyra/Lyra.uproject`)
+
+					if (m_ProjectStore->UpdateProject(ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath))
+					{
+						m_ProjectStats.ProjectWriteCount++;
+						ZEN_INFO("updated project (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
+								 ProjectId,
+								 Root,
+								 EngineRoot,
+								 ProjectRoot,
+								 ProjectFilePath,
+								 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
+
+						HttpReq.WriteResponse(HttpResponseCode::OK);
+					}
+					else
+					{
+						const std::filesystem::path BasePath = m_ProjectStore->BasePath() / ProjectId;
+						m_ProjectStore->NewProject(BasePath, ProjectId, Root, EngineRoot, ProjectRoot, ProjectFilePath);
+
+						m_ProjectStats.ProjectWriteCount++;
+						ZEN_INFO("established project (id: '{}', roots: '{}', '{}', '{}', '{}'{})",
+								 ProjectId,
+								 Root,
+								 EngineRoot,
+								 ProjectRoot,
+								 ProjectFilePath,
+								 ProjectFilePath.empty() ? ", project will not be GCd due to empty project file path" : "");
+
+						HttpReq.WriteResponse(HttpResponseCode::Created);
+					}
+				}
+				else
+				{
+					HttpReq.WriteResponse(HttpResponseCode::BadRequest,
+										  HttpContentType::kText,
+										  fmt::format("Malformed compact binary object: '{}'", ToString(ValidateResult)));
+				}
 			}
 			break;
 
diff --git a/src/zenserver/projectstore/jupiterremoteprojectstore.cpp b/src/zenserver/projectstore/jupiterremoteprojectstore.cpp
index d79ad3cb7..a8d486e2a 100644
--- a/src/zenserver/projectstore/jupiterremoteprojectstore.cpp
+++ b/src/zenserver/projectstore/jupiterremoteprojectstore.cpp
@@ -2,6 +2,7 @@
 
 #include "jupiterremoteprojectstore.h"
 
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compress.h>
 #include <zencore/fmtutils.h>
 
@@ -249,8 +250,9 @@ private:
 			return Result;
 		}
 
-		CbObject ContainerObject = LoadCompactBinaryObject(GetResult.Response);
-		if (!ContainerObject)
+		CbValidateError ValidateResult = CbValidateError::None;
+		if (CbObject ContainerObject = ValidateAndReadCompactBinaryObject(IoBuffer(GetResult.Response), ValidateResult);
+			ValidateResult != CbValidateError::None || !ContainerObject)
 		{
 			return LoadContainerResult{
 				RemoteProjectStore::Result{.ErrorCode	   = gsl::narrow<int32_t>(HttpResponseCode::InternalServerError),
@@ -262,7 +264,10 @@ private:
 																 Key)},
 				{}};
 		}
-		return LoadContainerResult{ConvertResult(GetResult), std::move(ContainerObject)};
+		else
+		{
+			return LoadContainerResult{ConvertResult(GetResult), std::move(ContainerObject)};
+		}
 	}
 
 	void AddStats(const JupiterResult& Result)
diff --git a/src/zenserver/projectstore/projectstore.cpp b/src/zenserver/projectstore/projectstore.cpp
index 56b64cee4..5277d689f 100644
--- a/src/zenserver/projectstore/projectstore.cpp
+++ b/src/zenserver/projectstore/projectstore.cpp
@@ -5439,67 +5439,70 @@ ProjectStore::WriteOplog(const std::string_view ProjectId, const std::string_vie
 	}
 	Project->TouchOplog(OplogId);
 
-	CbObject ContainerObject = LoadCompactBinaryObject(Payload);
-	if (!ContainerObject)
-	{
-		return {HttpResponseCode::BadRequest, "Invalid payload format"};
-	}
-
-	CidStore&							   ChunkStore = m_CidStore;
-	RwLock								   AttachmentsLock;
-	tsl::robin_set<IoHash, IoHash::Hasher> Attachments;
-
-	auto HasAttachment = [&ChunkStore](const IoHash& RawHash) { return ChunkStore.ContainsChunk(RawHash); };
-	auto OnNeedBlock   = [&AttachmentsLock, &Attachments](const IoHash& BlockHash, const std::vector<IoHash>&& ChunkHashes) {
-		  RwLock::ExclusiveLockScope _(AttachmentsLock);
-		  if (BlockHash != IoHash::Zero)
-		  {
-			  Attachments.insert(BlockHash);
-		  }
-		  else
-		  {
-			  Attachments.insert(ChunkHashes.begin(), ChunkHashes.end());
-		  }
-	};
-	auto OnNeedAttachment = [&AttachmentsLock, &Attachments](const IoHash& RawHash) {
-		RwLock::ExclusiveLockScope _(AttachmentsLock);
-		Attachments.insert(RawHash);
-	};
+	CbValidateError ValidateResult;
+	if (CbObject ContainerObject = ValidateAndReadCompactBinaryObject(std::move(Payload), ValidateResult);
+		ValidateResult == CbValidateError::None && ContainerObject)
+	{
+		CidStore&							   ChunkStore = m_CidStore;
+		RwLock								   AttachmentsLock;
+		tsl::robin_set<IoHash, IoHash::Hasher> Attachments;
+
+		auto HasAttachment = [&ChunkStore](const IoHash& RawHash) { return ChunkStore.ContainsChunk(RawHash); };
+		auto OnNeedBlock   = [&AttachmentsLock, &Attachments](const IoHash& BlockHash, const std::vector<IoHash>&& ChunkHashes) {
+			  RwLock::ExclusiveLockScope _(AttachmentsLock);
+			  if (BlockHash != IoHash::Zero)
+			  {
+				  Attachments.insert(BlockHash);
+			  }
+			  else
+			  {
+				  Attachments.insert(ChunkHashes.begin(), ChunkHashes.end());
+			  }
+		};
+		auto OnNeedAttachment = [&AttachmentsLock, &Attachments](const IoHash& RawHash) {
+			RwLock::ExclusiveLockScope _(AttachmentsLock);
+			Attachments.insert(RawHash);
+		};
 
-	auto OnChunkedAttachment = [](const ChunkedInfo&) {};
+		auto OnChunkedAttachment = [](const ChunkedInfo&) {};
 
-	auto OnReferencedAttachments = [&Oplog](std::span<IoHash> RawHashes) { Oplog->CaptureAddedAttachments(RawHashes); };
-	// Make sure we retain any attachments we download before writing the oplog
-	Oplog->EnableUpdateCapture();
-	auto _ = MakeGuard([&Oplog]() { Oplog->DisableUpdateCapture(); });
+		auto OnReferencedAttachments = [&Oplog](std::span<IoHash> RawHashes) { Oplog->CaptureAddedAttachments(RawHashes); };
+		// Make sure we retain any attachments we download before writing the oplog
+		Oplog->EnableUpdateCapture();
+		auto _ = MakeGuard([&Oplog]() { Oplog->DisableUpdateCapture(); });
 
-	RemoteProjectStore::Result RemoteResult = SaveOplogContainer(*Oplog,
-																 ContainerObject,
-																 OnReferencedAttachments,
-																 HasAttachment,
-																 OnNeedBlock,
-																 OnNeedAttachment,
-																 OnChunkedAttachment,
-																 nullptr);
+		RemoteProjectStore::Result RemoteResult = SaveOplogContainer(*Oplog,
+																	 ContainerObject,
+																	 OnReferencedAttachments,
+																	 HasAttachment,
+																	 OnNeedBlock,
+																	 OnNeedAttachment,
+																	 OnChunkedAttachment,
+																	 nullptr);
 
-	if (RemoteResult.ErrorCode)
-	{
-		return ConvertResult(RemoteResult);
-	}
+		if (RemoteResult.ErrorCode)
+		{
+			return ConvertResult(RemoteResult);
+		}
 
-	CbObjectWriter Cbo(1 + 1 + 5 + Attachments.size() * (1 + sizeof(IoHash::Hash)) + 1);
-	Cbo.BeginArray("need");
-	{
-		for (const IoHash& Hash : Attachments)
+		CbObjectWriter Cbo(1 + 1 + 5 + Attachments.size() * (1 + sizeof(IoHash::Hash)) + 1);
+		Cbo.BeginArray("need");
 		{
-			ZEN_DEBUG("Need attachment {}", Hash);
-			Cbo << Hash;
+			for (const IoHash& Hash : Attachments)
+			{
+				ZEN_DEBUG("Need attachment {}", Hash);
+				Cbo << Hash;
+			}
 		}
-	}
-	Cbo.EndArray();	 // "need"
+		Cbo.EndArray();	 // "need"
 
-	OutResponse = Cbo.Save();
-	return {HttpResponseCode::OK, {}};
+		OutResponse = Cbo.Save();
+		return {HttpResponseCode::OK, {}};
+	}
+	else
+	{
+		return {HttpResponseCode::BadRequest, fmt::format("Invalid payload format ('{}')", ToString(ValidateResult))};
+	}
 }
 
 std::pair<HttpResponseCode, std::string>
@@ -5604,15 +5607,19 @@ ProjectStore::Rpc(HttpServerRequest&	 HttpReq,
 			}
 			break;
 		case HttpContentType::kCbObject:
-			Cb = LoadCompactBinaryObject(Payload);
-			if (!Cb)
 			{
-				HttpReq.WriteResponse(HttpResponseCode::BadRequest,
-									  HttpContentType::kText,
-									  "Content format not supported, expected compact binary format");
-				return false;
+				CbValidateError ValidateResult;
+				if (Cb = ValidateAndReadCompactBinaryObject(std::move(Payload), ValidateResult);
+					ValidateResult != CbValidateError::None || !Cb)
+				{
+					HttpReq.WriteResponse(
+						HttpResponseCode::BadRequest,
+						HttpContentType::kText,
+						fmt::format("Content format not supported, expected compact binary format ('{}')", ToString(ValidateResult)));
+					return false;
+				}
+				break;
 			}
-			break;
 		case HttpContentType::kCbPackage:
 			try
 			{
@@ -5917,8 +5924,8 @@ ProjectStore::Rpc(HttpServerRequest&	 HttpReq,
 			ResponseObj.EndArray();
 		}
 
-		// Ops that have moved chunks to a compressed buffer for storage in m_CidStore have been rewritten with references to the new
-		// chunk(s). Make sure we add the chunks to m_CidStore, and do it after we update the oplog so GC doesn't think we have
+		// Ops that have moved chunks to a compressed buffer for storage in m_CidStore have been rewritten with references to the
+		// new chunk(s). Make sure we add the chunks to m_CidStore, and do it after we update the oplog so GC doesn't think we have
 		// unreferenced chunks.
 		for (auto It : AddedChunks)
 		{
@@ -7768,7 +7775,8 @@ TEST_CASE("project.store.gc.prep")
 		Project1->DeleteOplog("oplog1"sv);
 	}
 
-	// Caution - putting breakpoints and stepping through this part of the test likely makes it fails due to expiry time of pending chunks
+	// Caution - putting breakpoints and stepping through this part of the test likely makes it fails due to expiry time of pending
+	// chunks
 	{
 		Ref<ProjectStore::Project> Project1 = ProjectStore.OpenProject("proj1"sv);
 		Ref<ProjectStore::Oplog>   Oplog	= Project1->NewOplog("oplog1"sv, Project1OplogPath);
@@ -7790,8 +7798,8 @@ TEST_CASE("project.store.gc.prep")
 	}
 
 	Sleep(200);
-	// This pass they should also be retained since our age retention has kept them alive and they will now be picked up and the retention
-	// cleared
+	// This pass they should also be retained since our age retention has kept them alive and they will now be picked up and the
+	// retention cleared
 	{
 		GcSettings Settings = {.CacheExpireTime		   = GcClock::Now(),
 							   .ProjectStoreExpireTime = GcClock::Now(),
diff --git a/src/zenserver/projectstore/remoteprojectstore.cpp b/src/zenserver/projectstore/remoteprojectstore.cpp
index 857e868da..2a81ee3e3 100644
--- a/src/zenserver/projectstore/remoteprojectstore.cpp
+++ b/src/zenserver/projectstore/remoteprojectstore.cpp
@@ -2651,19 +2651,22 @@ ParseOplogContainer(const CbObject&																	  ContainerObject,
 	IoBuffer   OpsBuffer(IoBuffer::Wrap, OpsSection.GetData(), OpsSection.GetSize());
 	IoBuffer   SectionPayload = CompressedBuffer::FromCompressedNoValidate(std::move(OpsBuffer)).Decompress().AsIoBuffer();
 
+	CbValidateError ValidateResult = CbValidateError::None;
+	if (CbObject SectionObject = ValidateAndReadCompactBinaryObject(std::move(SectionPayload), ValidateResult);
+		ValidateResult == CbValidateError::None && ContainerObject)
 	{
-		CbObject SectionObject = LoadCompactBinaryObject(SectionPayload);
-		if (!SectionObject)
-		{
-			remotestore_impl::ReportMessage(OptionalContext,
-											fmt::format("Failed to save oplog container: '{}'", "Section has unexpected data type"));
-			return RemoteProjectStore::Result{gsl::narrow<int>(HttpResponseCode::BadRequest),
-											  Timer.GetElapsedTimeMs() / 1000.0,
-											  "Section has unexpected data type",
-											  "Failed to save oplog container"};
-		}
 		OutOplogSection = SectionObject;
 	}
+	else
+	{
+		remotestore_impl::ReportMessage(
+			OptionalContext,
+			fmt::format("Failed to save oplog container: '{}' ('{}')", "Section has unexpected data type", ToString(ValidateResult)));
+		return RemoteProjectStore::Result{gsl::narrow<int>(HttpResponseCode::BadRequest),
+										  Timer.GetElapsedTimeMs() / 1000.0,
+										  "Section has unexpected data type",
+										  "Failed to save oplog container"};
+	}
 	std::unordered_set<IoHash, IoHash::Hasher> OpsAttachments;
 	{
 		CbArrayView OpsArray = OutOplogSection["ops"sv].AsArrayView();