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

* add validation of compact binary payloads before reading them

1 files changed, 117 insertions, 87 deletions

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;