Add validation tests and harden legacy package parser

- Add tests for ParsePackageMessage validation: truncated header, bad
  magic, attachment count overflow, truncated tables/data, local ref
  rejection, and CbPackageReader equivalents
- Add tests for legacy::TryLoadCbPackage: empty input, zero-size
  binary rejection, null mapper handling, hash validation toggle
- Harden legacy parser: reject zero-size binary fields that would
  desync the reader, replace ZEN_ASSERT(Mapper) with graceful failure,
  add optional ValidateHashes parameter for resolved attachment data
- Rename test cases from "usonpackage.*" to "cbpackage.*"
- Fix CbPackage.LocalRef test to pass kAllowLocalReferences

1 files changed, 112 insertions, 8 deletions

diff --git a/src/zencore/compactbinarypackage.cpp b/src/zencore/compactbinarypackage.cpp
index 56a292ca6..76a18e9ed 100644
--- a/src/zencore/compactbinarypackage.cpp
+++ b/src/zencore/compactbinarypackage.cpp
@@ -684,14 +684,22 @@ namespace legacy {
 		Writer.Save(Ar);
 	}
 
-	bool TryLoadCbPackage(CbPackage& Package, IoBuffer InBuffer, BufferAllocator Allocator, CbPackage::AttachmentResolver* Mapper)
+	bool TryLoadCbPackage(CbPackage&					 Package,
+						  IoBuffer						 InBuffer,
+						  BufferAllocator				 Allocator,
+						  CbPackage::AttachmentResolver* Mapper,
+						  bool							 ValidateHashes)
 	{
 		BinaryReader Reader(InBuffer.Data(), InBuffer.Size());
 
-		return TryLoadCbPackage(Package, Reader, Allocator, Mapper);
+		return TryLoadCbPackage(Package, Reader, Allocator, Mapper, ValidateHashes);
 	}
 
-	bool TryLoadCbPackage(CbPackage& Package, BinaryReader& Reader, BufferAllocator Allocator, CbPackage::AttachmentResolver* Mapper)
+	bool TryLoadCbPackage(CbPackage&					 Package,
+						  BinaryReader&					 Reader,
+						  BufferAllocator				 Allocator,
+						  CbPackage::AttachmentResolver* Mapper,
+						  bool							 ValidateHashes)
 	{
 		Package = CbPackage();
 		for (;;)
@@ -708,7 +716,11 @@ namespace legacy {
 			if (ValueField.IsBinary())
 			{
 				const MemoryView View = ValueField.AsBinaryView();
-				if (View.GetSize() > 0)
+				if (View.GetSize() == 0)
+				{
+					return false;
+				}
+				else
 				{
 					SharedBuffer  Buffer	= SharedBuffer::MakeView(View, ValueField.GetOuterBuffer()).MakeOwned();
 					CbField		  HashField = LoadCompactBinary(Reader, Allocator);
@@ -748,7 +760,11 @@ namespace legacy {
 			{
 				const IoHash Hash = ValueField.AsHash();
 
-				ZEN_ASSERT(Mapper);
+				if (!Mapper)
+				{
+					return false;
+				}
+
 				if (SharedBuffer AttachmentData = (*Mapper)(Hash))
 				{
 					IoHash	 RawHash;
@@ -763,6 +779,10 @@ namespace legacy {
 					}
 					else
 					{
+						if (ValidateHashes && IoHash::HashBuffer(AttachmentData) != Hash)
+						{
+							return false;
+						}
 						const CbValidateError ValidationResult = ValidateCompactBinary(AttachmentData.GetView(), CbValidateMode::All);
 						if (ValidationResult != CbValidateError::None)
 						{
@@ -807,7 +827,7 @@ usonpackage_forcelink()
 
 TEST_SUITE_BEGIN("core.compactbinarypackage");
 
-TEST_CASE("usonpackage")
+TEST_CASE("cbpackage")
 {
 	using namespace std::literals;
 
@@ -997,7 +1017,7 @@ TEST_CASE("usonpackage")
 	}
 }
 
-TEST_CASE("usonpackage.serialization")
+TEST_CASE("cbpackage.serialization")
 {
 	using namespace std::literals;
 
@@ -1303,7 +1323,7 @@ TEST_CASE("usonpackage.serialization")
 	}
 }
 
-TEST_CASE("usonpackage.invalidpackage")
+TEST_CASE("cbpackage.invalidpackage")
 {
 	const auto TestLoad = [](std::initializer_list<uint8_t> RawData, BufferAllocator Allocator = UniqueBuffer::Alloc) {
 		const MemoryView RawView = MakeMemoryView(RawData);
@@ -1345,6 +1365,90 @@ TEST_CASE("usonpackage.invalidpackage")
 	}
 }
 
+TEST_CASE("cbpackage.legacy.invalidpackage")
+{
+	const auto TestLegacyLoad = [](std::initializer_list<uint8_t> RawData) {
+		const MemoryView RawView = MakeMemoryView(RawData);
+		IoBuffer		 Buffer(IoBuffer::Wrap, const_cast<void*>(RawView.GetData()), RawView.GetSize());
+		CbPackage		 Package;
+		CHECK_FALSE(legacy::TryLoadCbPackage(Package, Buffer, &UniqueBuffer::Alloc));
+	};
+
+	SUBCASE("Empty") { TestLegacyLoad({}); }
+
+	SUBCASE("Zero size binary rejects")
+	{
+		// A binary field with zero payload size should be rejected (would desync the reader)
+		BinaryWriter Writer;
+		CbWriter	 Cb;
+		Cb.AddBinary(MemoryView());	 // zero-size binary
+		Cb.Save(Writer);
+
+		IoBuffer  Buffer(IoBuffer::Wrap, const_cast<void*>(MakeMemoryView(Writer).GetData()), MakeMemoryView(Writer).GetSize());
+		CbPackage Package;
+		CHECK_FALSE(legacy::TryLoadCbPackage(Package, Buffer, &UniqueBuffer::Alloc));
+	}
+}
+
+TEST_CASE("cbpackage.legacy.hashresolution")
+{
+	// Build a valid legacy package with an object, then round-trip it
+	CbObjectWriter RootWriter;
+	RootWriter.AddString("name", "test");
+	CbObject RootObject = RootWriter.Save();
+
+	CbAttachment ObjectAttach(RootObject);
+
+	CbPackage OriginalPkg;
+	OriginalPkg.SetObject(RootObject);
+	OriginalPkg.AddAttachment(ObjectAttach);
+
+	BinaryWriter Writer;
+	legacy::SaveCbPackage(OriginalPkg, Writer);
+
+	IoBuffer  Buffer(IoBuffer::Wrap, const_cast<void*>(MakeMemoryView(Writer).GetData()), MakeMemoryView(Writer).GetSize());
+	CbPackage LoadedPkg;
+	CHECK(legacy::TryLoadCbPackage(LoadedPkg, Buffer, &UniqueBuffer::Alloc));
+
+	// The hash-only path requires a mapper — without one it should fail
+	CbWriter HashOnlyCb;
+	HashOnlyCb.AddHash(ObjectAttach.GetHash());
+	HashOnlyCb.AddNull();
+	BinaryWriter HashOnlyWriter;
+	HashOnlyCb.Save(HashOnlyWriter);
+
+	IoBuffer  HashOnlyBuffer(IoBuffer::Wrap,
+							 const_cast<void*>(MakeMemoryView(HashOnlyWriter).GetData()),
+							 MakeMemoryView(HashOnlyWriter).GetSize());
+	CbPackage HashOnlyPkg;
+	CHECK_FALSE(legacy::TryLoadCbPackage(HashOnlyPkg, HashOnlyBuffer, &UniqueBuffer::Alloc, nullptr));
+
+	// With a mapper that returns valid data, it should succeed
+	CbPackage::AttachmentResolver Resolver = [&](const IoHash& Hash) -> SharedBuffer {
+		if (Hash == ObjectAttach.GetHash())
+		{
+			return RootObject.GetBuffer();
+		}
+		return {};
+	};
+	CHECK(legacy::TryLoadCbPackage(HashOnlyPkg, HashOnlyBuffer, &UniqueBuffer::Alloc, &Resolver));
+
+	// Build a different but structurally valid CbObject to use as mismatched data
+	CbObjectWriter DifferentWriter;
+	DifferentWriter.AddString("name", "different");
+	CbObject DifferentObject = DifferentWriter.Save();
+
+	CbPackage::AttachmentResolver BadResolver = [&](const IoHash&) -> SharedBuffer { return DifferentObject.GetBuffer(); };
+	CbPackage					  BadPkg;
+
+	// With ValidateHashes enabled and a mapper that returns mismatched data, it should fail
+	CHECK_FALSE(legacy::TryLoadCbPackage(BadPkg, HashOnlyBuffer, &UniqueBuffer::Alloc, &BadResolver, /*ValidateHashes*/ true));
+
+	// Without ValidateHashes, the mismatched data is accepted (structure is still valid CB)
+	CbPackage UncheckedPkg;
+	CHECK(legacy::TryLoadCbPackage(UncheckedPkg, HashOnlyBuffer, &UniqueBuffer::Alloc, &BadResolver, /*ValidateHashes*/ false));
+}
+
 TEST_SUITE_END();
 
 #endif