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

* add validation of compact binary payloads before reading them

4 files changed, 103 insertions, 46 deletions

diff --git a/src/zenhttp/auth/authmgr.cpp b/src/zenhttp/auth/authmgr.cpp
index 6c1a66a99..209276621 100644
--- a/src/zenhttp/auth/authmgr.cpp
+++ b/src/zenhttp/auth/authmgr.cpp
@@ -5,7 +5,7 @@
 #include <zencore/basicfile.h>
 #include <zencore/compactbinary.h>
 #include <zencore/compactbinarybuilder.h>
-#include <zencore/compactbinaryvalidation.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/crypto.h>
 #include <zencore/filesystem.h>
 #include <zencore/logging.h>
@@ -297,15 +297,14 @@ private:
 				return;
 			}
 
-			const CbValidateError ValidationError = ValidateCompactBinary(Buffer, CbValidateMode::All);
-
-			if (ValidationError != CbValidateError::None)
+			CbValidateError ValidationError;
+			if (CbObject AuthState = ValidateAndReadCompactBinaryObject(std::move(Buffer), ValidationError);
+				ValidationError != CbValidateError::None)
 			{
-				ZEN_WARN("load serialized state FAILED, reason 'Invalid compact binary'");
+				ZEN_WARN("load serialized state FAILED, reason '{}'", ToString(ValidationError));
 				return;
 			}
-
-			if (CbObject AuthState = LoadCompactBinaryObject(Buffer))
+			else
 			{
 				for (CbFieldView ProviderView : AuthState["OpenIdProviders"sv])
 				{
diff --git a/src/zenhttp/httpclient.cpp b/src/zenhttp/httpclient.cpp
index 30a2bfc65..9ee8cc05a 100644
--- a/src/zenhttp/httpclient.cpp
+++ b/src/zenhttp/httpclient.cpp
@@ -7,6 +7,7 @@
 
 #include <zencore/compactbinarybuilder.h>
 #include <zencore/compactbinarypackage.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compositebuffer.h>
 #include <zencore/except.h>
 #include <zencore/filesystem.h>
@@ -984,13 +985,16 @@ HttpClient::TransactPackage(std::string_view Url, CbPackage Package, const KeyVa
 
 		if (FilterResponse.status_code == 200)
 		{
-			IoBuffer ResponseBuffer(IoBuffer::Wrap, FilterResponse.text.data(), FilterResponse.text.size());
-			CbObject ResponseObject = LoadCompactBinaryObject(ResponseBuffer);
-
-			for (CbFieldView& Entry : ResponseObject["need"])
+			IoBuffer		ResponseBuffer(IoBuffer::Wrap, FilterResponse.text.data(), FilterResponse.text.size());
+			CbValidateError ValidationError = CbValidateError::None;
+			if (CbObject ResponseObject = ValidateAndReadCompactBinaryObject(std::move(ResponseBuffer), ValidationError);
+				ValidationError == CbValidateError::None)
 			{
-				ZEN_ASSERT(Entry.IsHash());
-				AttachmentsToSend.push_back(Entry.AsHash());
+				for (CbFieldView& Entry : ResponseObject["need"])
+				{
+					ZEN_ASSERT(Entry.IsHash());
+					AttachmentsToSend.push_back(Entry.AsHash());
+				}
 			}
 		}
 	}
@@ -1550,11 +1554,14 @@ HttpClient::Download(std::string_view Url, const std::filesystem::path& TempFold
 CbObject
 HttpClient::Response::AsObject() const
 {
-	// TODO: sanity check the payload format etc
-
 	if (ResponsePayload)
 	{
-		return LoadCompactBinaryObject(ResponsePayload);
+		CbValidateError ValidationError = CbValidateError::None;
+		if (CbObject ResponseObject = ValidateAndReadCompactBinaryObject(IoBuffer(ResponsePayload), ValidationError);
+			ValidationError == CbValidateError::None)
+		{
+			return ResponseObject;
+		}
 	}
 
 	return {};
diff --git a/src/zenhttp/httpserver.cpp b/src/zenhttp/httpserver.cpp
index 764f2a2a7..2c063d646 100644
--- a/src/zenhttp/httpserver.cpp
+++ b/src/zenhttp/httpserver.cpp
@@ -18,6 +18,7 @@
 #include <zencore/compactbinary.h>
 #include <zencore/compactbinarybuilder.h>
 #include <zencore/compactbinarypackage.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/iobuffer.h>
 #include <zencore/logging.h>
 #include <zencore/stream.h>
@@ -665,9 +666,13 @@ HttpServerRequest::ReadPayloadObject()
 			}
 			return CbObject();
 		}
-		return LoadCompactBinaryObject(std::move(Payload));
+		CbValidateError ValidationError = CbValidateError::None;
+		if (CbObject ResponseObject = ValidateAndReadCompactBinaryObject(std::move(Payload), ValidationError);
+			ValidationError == CbValidateError::None)
+		{
+			return ResponseObject;
+		}
 	}
-
 	return {};
 }
 
@@ -931,42 +936,51 @@ HandlePackageOffers(HttpService& Service, HttpServerRequest& Request, Ref<IHttpP
 
 			if (PackageHandlerRef)
 			{
-				CbObject OfferMessage = LoadCompactBinaryObject(Request.ReadPayload());
-
-				std::vector<IoHash> OfferCids;
-
-				for (auto& CidEntry : OfferMessage["offer"])
+				CbValidateError ValidationError = CbValidateError::None;
+				if (CbObject OfferMessage = ValidateAndReadCompactBinaryObject(IoBuffer(Request.ReadPayload()), ValidationError);
+					ValidationError == CbValidateError::None)
 				{
-					if (!CidEntry.IsHash())
+					std::vector<IoHash> OfferCids;
+
+					for (auto& CidEntry : OfferMessage["offer"])
 					{
-						// Should yield bad request response?
+						if (!CidEntry.IsHash())
+						{
+							// Should yield bad request response?
+
+							ZEN_WARN("found invalid entry in offer");
 
-						ZEN_WARN("found invalid entry in offer");
+							continue;
+						}
 
-						continue;
+						OfferCids.push_back(CidEntry.AsHash());
 					}
 
-					OfferCids.push_back(CidEntry.AsHash());
-				}
+					ZEN_TRACE("request #{} -> filtering offer of {} entries", Request.RequestId(), OfferCids.size());
+
+					PackageHandlerRef->FilterOffer(OfferCids);
 
-				ZEN_TRACE("request #{} -> filtering offer of {} entries", Request.RequestId(), OfferCids.size());
+					ZEN_TRACE("request #{} -> filtered to {} entries", Request.RequestId(), OfferCids.size());
 
-				PackageHandlerRef->FilterOffer(OfferCids);
+					CbObjectWriter ResponseWriter;
+					ResponseWriter.BeginArray("need");
 
-				ZEN_TRACE("request #{} -> filtered to {} entries", Request.RequestId(), OfferCids.size());
+					for (const IoHash& Cid : OfferCids)
+					{
+						ResponseWriter.AddHash(Cid);
+					}
 
-				CbObjectWriter ResponseWriter;
-				ResponseWriter.BeginArray("need");
+					ResponseWriter.EndArray();
 
-				for (const IoHash& Cid : OfferCids)
+					// Emit filter response
+					Request.WriteResponse(HttpResponseCode::OK, ResponseWriter.Save());
+				}
+				else
 				{
-					ResponseWriter.AddHash(Cid);
+					Request.WriteResponse(HttpResponseCode::BadRequest,
+										  HttpContentType::kText,
+										  fmt::format("Invalid request payload: '{}'", ToString(ValidationError)));
 				}
-
-				ResponseWriter.EndArray();
-
-				// Emit filter response
-				Request.WriteResponse(HttpResponseCode::OK, ResponseWriter.Save());
 				return true;
 			}
 		}
diff --git a/src/zenhttp/packageformat.cpp b/src/zenhttp/packageformat.cpp
index 0b7848f79..f622b93ea 100644
--- a/src/zenhttp/packageformat.cpp
+++ b/src/zenhttp/packageformat.cpp
@@ -4,6 +4,7 @@
 
 #include <zencore/compactbinarybuilder.h>
 #include <zencore/compactbinarypackage.h>
+#include <zencore/compactbinaryutil.h>
 #include <zencore/compositebuffer.h>
 #include <zencore/filesystem.h>
 #include <zencore/fmtutils.h>
@@ -499,6 +500,8 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 		{
 			if (Entry.Flags & CbAttachmentEntry::kIsObject)
 			{
+				CbObject AttachmentObject;
+
 				CompressedBuffer CompBuf(CompressedBuffer::FromCompressedNoValidate(IoBuffer(AttachmentBuffer)));
 				if (!CompBuf)
 				{
@@ -509,7 +512,18 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 												   AttachmentBuffer.GetSize(),
 												   Entry.AttachmentHash)));
 				}
-				CbObject AttachmentObject = LoadCompactBinaryObject(std::move(CompBuf));
+				else
+				{
+					CbValidateError ValidationError = CbValidateError::None;
+					AttachmentObject				= ValidateAndReadCompactBinaryObject(std::move(CompBuf), ValidationError);
+					if (ValidationError != CbValidateError::None)
+					{
+						MalformedAttachments.push_back(std::make_pair(
+							i,
+							fmt::format("Invalid format, CbObject for {}. Reason '{}'", Entry.AttachmentHash, ToString(ValidationError))));
+					}
+				}
+
 				if (i == 0)
 				{
 					// First payload is always a compact binary object
@@ -541,7 +555,15 @@ ParsePackageMessage(IoBuffer Payload, std::function<IoBuffer(const IoHash&, uint
 		{
 			if (Entry.Flags & CbAttachmentEntry::kIsObject)
 			{
-				CbObject AttachmentObject = LoadCompactBinaryObject(AttachmentBuffer);
+				CbValidateError ValidationError	 = CbValidateError::None;
+				CbObject		AttachmentObject = ValidateAndReadCompactBinaryObject(std::move(AttachmentBuffer), ValidationError);
+				if (ValidationError != CbValidateError::None)
+				{
+					MalformedAttachments.push_back(std::make_pair(
+						i,
+						fmt::format("Invalid format, CbObject for {}. Reason '{}'", Entry.AttachmentHash, ToString(ValidationError))));
+				}
+
 				if (i == 0)
 				{
 					Package.SetObject(AttachmentObject);
@@ -709,7 +731,12 @@ CbPackageReader::Finalize()
 			{
 				if (Entry.Flags & CbAttachmentEntry::kIsLocalRef)
 				{
-					m_RootObject = LoadCompactBinaryObject(MarshalLocalChunkReference(AttachmentBuffer));
+					CbValidateError ValidateError = CbValidateError::None;
+					m_RootObject = ValidateAndReadCompactBinaryObject(MarshalLocalChunkReference(AttachmentBuffer), ValidateError);
+					if (ValidateError != CbValidateError::None)
+					{
+						throw std::runtime_error(fmt::format("Root object format is invalid, reason: '{}'", ToString(ValidateError)));
+					}
 				}
 				else if (Entry.Flags & CbAttachmentEntry::kIsCompressed)
 				{
@@ -718,12 +745,22 @@ CbPackageReader::Finalize()
 					CompressedBuffer Compressed = CompressedBuffer::FromCompressed(SharedBuffer(AttachmentBuffer), RawHash, RawSize);
 					if (RawHash == Entry.AttachmentHash)
 					{
-						m_RootObject = LoadCompactBinaryObject(Compressed);
+						CbValidateError ValidateError = CbValidateError::None;
+						m_RootObject				  = ValidateAndReadCompactBinaryObject(std::move(Compressed), ValidateError);
+						if (ValidateError != CbValidateError::None)
+						{
+							throw std::runtime_error(fmt::format("Root object format is invalid, reason: '{}'", ToString(ValidateError)));
+						}
 					}
 				}
 				else
 				{
-					m_RootObject = LoadCompactBinaryObject(std::move(AttachmentBuffer));
+					CbValidateError ValidateError = CbValidateError::None;
+					m_RootObject				  = ValidateAndReadCompactBinaryObject(std::move(AttachmentBuffer), ValidateError);
+					if (ValidateError != CbValidateError::None)
+					{
+						throw std::runtime_error(fmt::format("Root object format is invalid, reason: '{}'", ToString(ValidateError)));
+					}
 				}
 			}
 			else