Validate serialization during Contract deploy and Update (#2948)

* Fix 2899

* Optimize

* Add UT

* UT manifest

* Add failure ut for testing

* Possible fix

* Prevent future issues

* Prevent deploy

* Check items during serialization to avoid serialize and deserialize

* Fix comment

* Clean code

* Clean changes

* Move condition

* Fix exception message

src/Neo/SmartContract/ApplicationEngine.Runtime.cs

@@ -362,7 +362,7 @@ protected internal void RuntimeNotify(byte[] eventName, Array state)
             }
             using MemoryStream ms = new(MaxNotificationSize);
             using BinaryWriter writer = new(ms, Utility.StrictUTF8, true);
-            BinarySerializer.Serialize(writer, state, MaxNotificationSize);
+            BinarySerializer.Serialize(writer, state, MaxNotificationSize, Limits.MaxStackSize);
             SendNotification(CurrentScriptHash, name, state);
         }
 
@@ -373,7 +373,7 @@ protected internal void RuntimeNotifyV1(byte[] eventName, Array state)
                 throw new InvalidOperationException("Notifications are not allowed in dynamic scripts.");
             using MemoryStream ms = new(MaxNotificationSize);
             using BinaryWriter writer = new(ms, Utility.StrictUTF8, true);
-            BinarySerializer.Serialize(writer, state, MaxNotificationSize);
+            BinarySerializer.Serialize(writer, state, MaxNotificationSize, Limits.MaxStackSize);
             SendNotification(CurrentScriptHash, Utility.StrictUTF8.GetString(eventName), state);
         }
 

src/Neo/SmartContract/BinarySerializer.cs

@@ -8,14 +8,14 @@
 // Redistribution and use in source and binary forms with or without
 // modifications are permitted.
 
-using Neo.IO;
-using Neo.VM;
-using Neo.VM.Types;
 using System;
 using System.Collections.Generic;
 using System.IO;
 using System.Linq;
 using System.Numerics;
+using Neo.IO;
+using Neo.VM;
+using Neo.VM.Types;
 using Array = Neo.VM.Types.Array;
 using Boolean = Neo.VM.Types.Boolean;
 using Buffer = Neo.VM.Types.Buffer;
@@ -55,7 +55,7 @@ public ContainerPlaceholder(StackItemType type, int count)
         public static StackItem Deserialize(ReadOnlyMemory<byte> data, ExecutionEngineLimits limits, ReferenceCounter referenceCounter = null)
         {
             MemoryReader reader = new(data);
-            return Deserialize(ref reader, limits with { MaxItemSize = (uint)data.Length }, referenceCounter);
+            return Deserialize(ref reader, (uint)Math.Min(data.Length, limits.MaxItemSize), limits.MaxStackSize, referenceCounter);
         }
 
         /// <summary>
@@ -65,7 +65,20 @@ public static StackItem Deserialize(ReadOnlyMemory<byte> data, ExecutionEngineLi
         /// <param name="limits">The limits for the deserialization.</param>
         /// <param name="referenceCounter">The <see cref="ReferenceCounter"/> used by the <see cref="StackItem"/>.</param>
         /// <returns>The deserialized <see cref="StackItem"/>.</returns>
-        public static StackItem Deserialize(ref MemoryReader reader, ExecutionEngineLimits limits, ReferenceCounter referenceCounter)
+        public static StackItem Deserialize(ref MemoryReader reader, ExecutionEngineLimits limits, ReferenceCounter referenceCounter = null)
+        {
+            return Deserialize(ref reader, limits.MaxItemSize, limits.MaxStackSize, referenceCounter);
+        }
+
+        /// <summary>
+        /// Deserializes a <see cref="StackItem"/> from <see cref="MemoryReader"/>.
+        /// </summary>
+        /// <param name="reader">The <see cref="MemoryReader"/> for reading data.</param>
+        /// <param name="maxSize">The maximum size of the result.</param>
+        /// <param name="maxItems">The max of items to serialize</param>
+        /// <param name="referenceCounter">The <see cref="ReferenceCounter"/> used by the <see cref="StackItem"/>.</param>
+        /// <returns>The deserialized <see cref="StackItem"/>.</returns>
+        public static StackItem Deserialize(ref MemoryReader reader, uint maxSize, uint maxItems, ReferenceCounter referenceCounter = null)
         {
             Stack<StackItem> deserialized = new();
             int undeserialized = 1;
@@ -84,31 +97,32 @@ public static StackItem Deserialize(ref MemoryReader reader, ExecutionEngineLimi
                         deserialized.Push(new BigInteger(reader.ReadVarMemory(Integer.MaxSize).Span));
                         break;
                     case StackItemType.ByteString:
-                        deserialized.Push(reader.ReadVarMemory((int)limits.MaxItemSize));
+                        deserialized.Push(reader.ReadVarMemory((int)maxSize));
                         break;
                     case StackItemType.Buffer:
-                        ReadOnlyMemory<byte> memory = reader.ReadVarMemory((int)limits.MaxItemSize);
+                        ReadOnlyMemory<byte> memory = reader.ReadVarMemory((int)maxSize);
                         deserialized.Push(new Buffer(memory.Span));
                         break;
                     case StackItemType.Array:
                     case StackItemType.Struct:
                         {
-                            int count = (int)reader.ReadVarInt(limits.MaxStackSize);
+                            int count = (int)reader.ReadVarInt(maxItems);
                             deserialized.Push(new ContainerPlaceholder(type, count));
                             undeserialized += count;
                         }
                         break;
                     case StackItemType.Map:
                         {
-                            int count = (int)reader.ReadVarInt(limits.MaxStackSize);
+                            int count = (int)reader.ReadVarInt(maxItems);
                             deserialized.Push(new ContainerPlaceholder(type, count));
                             undeserialized += count * 2;
                         }
                         break;
                     default:
                         throw new FormatException();
                 }
-                if (deserialized.Count > limits.MaxStackSize) throw new FormatException();
+                if (deserialized.Count > maxItems)
+                    throw new FormatException();
             }
             Stack<StackItem> stack_temp = new();
             while (deserialized.Count > 0)
@@ -147,17 +161,29 @@ public static StackItem Deserialize(ref MemoryReader reader, ExecutionEngineLimi
             return stack_temp.Peek();
         }
 
+        /// <summary>
+        /// Serializes a <see cref="StackItem"/> to byte array.
+        /// </summary>
+        /// <param name="item">The <see cref="StackItem"/> to be serialized.</param>
+        /// <param name="limits">The <see cref="ExecutionEngineLimits"/> used to ensure the limits.</param>
+        /// <returns>The serialized byte array.</returns>
+        public static byte[] Serialize(StackItem item, ExecutionEngineLimits limits)
+        {
+            return Serialize(item, limits.MaxItemSize, limits.MaxStackSize);
+        }
+
         /// <summary>
         /// Serializes a <see cref="StackItem"/> to byte array.
         /// </summary>
         /// <param name="item">The <see cref="StackItem"/> to be serialized.</param>
         /// <param name="maxSize">The maximum size of the result.</param>
+        /// <param name="maxItems">The max of items to serialize</param>
         /// <returns>The serialized byte array.</returns>
-        public static byte[] Serialize(StackItem item, uint maxSize)
+        public static byte[] Serialize(StackItem item, long maxSize, long maxItems)
         {
             using MemoryStream ms = new();
             using BinaryWriter writer = new(ms, Utility.StrictUTF8, true);
-            Serialize(writer, item, maxSize);
+            Serialize(writer, item, maxSize, maxItems);
             writer.Flush();
             return ms.ToArray();
         }
@@ -168,13 +194,16 @@ public static byte[] Serialize(StackItem item, uint maxSize)
         /// <param name="writer">The <see cref="BinaryWriter"/> for writing data.</param>
         /// <param name="item">The <see cref="StackItem"/> to be serialized.</param>
         /// <param name="maxSize">The maximum size of the result.</param>
-        public static void Serialize(BinaryWriter writer, StackItem item, uint maxSize)
+        /// <param name="maxItems">The max of items to serialize</param>
+        public static void Serialize(BinaryWriter writer, StackItem item, long maxSize, long maxItems)
         {
             HashSet<CompoundType> serialized = new(ReferenceEqualityComparer.Instance);
             Stack<StackItem> unserialized = new();
             unserialized.Push(item);
             while (unserialized.Count > 0)
             {
+                if (--maxItems < 0)
+                    throw new FormatException();
                 item = unserialized.Pop();
                 writer.Write((byte)item.Type);
                 switch (item)

src/Neo/SmartContract/Manifest/ContractManifest.cs

@@ -8,12 +8,12 @@
 // Redistribution and use in source and binary forms with or without
 // modifications are permitted.
 
+using System;
+using System.Linq;
 using Neo.IO;
 using Neo.Json;
 using Neo.VM;
 using Neo.VM.Types;
-using System;
-using System.Linq;
 using Array = Neo.VM.Types.Array;
 
 namespace Neo.SmartContract.Manifest
@@ -172,10 +172,21 @@ public JObject ToJson()
         /// <summary>
         /// Determines whether the manifest is valid.
         /// </summary>
+        /// <param name="limits">The <see cref="ExecutionEngineLimits"/> used for test serialization.</param>
         /// <param name="hash">The hash of the contract.</param>
         /// <returns><see langword="true"/> if the manifest is valid; otherwise, <see langword="false"/>.</returns>
-        public bool IsValid(UInt160 hash)
+        public bool IsValid(ExecutionEngineLimits limits, UInt160 hash)
         {
+            // Ensure that is serializable
+            try
+            {
+                _ = BinarySerializer.Serialize(ToStackItem(null), limits);
+            }
+            catch
+            {
+                return false;
+            }
+            // Check groups
             return Groups.All(u => u.IsValid(hash));
         }
     }

src/Neo/SmartContract/Native/ContractManagement.cs

@@ -10,17 +10,18 @@
 
 #pragma warning disable IDE0051
 
+using System;
+using System.Buffers.Binary;
+using System.Collections.Generic;
+using System.Linq;
+using System.Numerics;
 using Neo.IO;
 using Neo.Network.P2P.Payloads;
 using Neo.Persistence;
 using Neo.SmartContract.Iterators;
 using Neo.SmartContract.Manifest;
+using Neo.VM;
 using Neo.VM.Types;
-using System;
-using System.Buffers.Binary;
-using System.Collections.Generic;
-using System.Linq;
-using System.Numerics;
 
 namespace Neo.SmartContract.Native
 {
@@ -249,7 +250,7 @@ private async ContractTask<ContractState> Deploy(ApplicationEngine engine, byte[
                 Manifest = parsedManifest
             };
 
-            if (!contract.Manifest.IsValid(hash)) throw new InvalidOperationException($"Invalid Manifest Hash: {hash}");
+            if (!contract.Manifest.IsValid(engine.Limits, hash)) throw new InvalidOperationException($"Invalid Manifest: {hash}");
 
             engine.Snapshot.Add(key, new StorageItem(contract));
             engine.Snapshot.Add(CreateStorageKey(Prefix_ContractHash).AddBigEndian(contract.Id), new StorageItem(hash.ToArray()));
@@ -291,8 +292,8 @@ private ContractTask Update(ApplicationEngine engine, byte[] nefFile, byte[] man
                 ContractManifest manifest_new = ContractManifest.Parse(manifest);
                 if (manifest_new.Name != contract.Manifest.Name)
                     throw new InvalidOperationException("The name of the contract can't be changed.");
-                if (!manifest_new.IsValid(contract.Hash))
-                    throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}");
+                if (!manifest_new.IsValid(engine.Limits, contract.Hash))
+                    throw new InvalidOperationException($"Invalid Manifest: {contract.Hash}");
                 contract.Manifest = manifest_new;
             }
             Helper.Check(new VM.Script(contract.Nef.Script, engine.IsHardforkEnabled(Hardfork.HF_Basilisk)), contract.Manifest.Abi);

src/Neo/SmartContract/Native/OracleContract.cs

@@ -255,7 +255,7 @@ private async ContractTask Request(ApplicationEngine engine, string url, string
                 Filter = filter,
                 CallbackContract = engine.CallingScriptHash,
                 CallbackMethod = callback,
-                UserData = BinarySerializer.Serialize(userData, MaxUserDataLength)
+                UserData = BinarySerializer.Serialize(userData, MaxUserDataLength, engine.Limits.MaxStackSize)
             }));
 
             //Add the id to the IdList

src/Neo/SmartContract/Native/StdLib.cs

@@ -10,13 +10,12 @@
 
 #pragma warning disable IDE0051
 
-using Neo.Cryptography;
-using Neo.Json;
-using Neo.VM.Types;
 using System;
 using System.Globalization;
 using System.Numerics;
-using System.Text;
+using Neo.Cryptography;
+using Neo.Json;
+using Neo.VM.Types;
 
 namespace Neo.SmartContract.Native
 {
@@ -32,7 +31,7 @@ internal StdLib() { }
         [ContractMethod(CpuFee = 1 << 12)]
         private static byte[] Serialize(ApplicationEngine engine, StackItem item)
         {
-            return BinarySerializer.Serialize(item, engine.Limits.MaxItemSize);
+            return BinarySerializer.Serialize(item, engine.Limits);
         }
 
         [ContractMethod(CpuFee = 1 << 14)]

src/Neo/SmartContract/StorageItem.cs

@@ -8,11 +8,11 @@
 // Redistribution and use in source and binary forms with or without
 // modifications are permitted.
 
-using Neo.IO;
-using Neo.VM;
 using System;
 using System.IO;
 using System.Numerics;
+using Neo.IO;
+using Neo.VM;
 
 namespace Neo.SmartContract
 {
@@ -36,7 +36,7 @@ public ReadOnlyMemory<byte> Value
                 return !value.IsEmpty ? value : value = cache switch
                 {
                     BigInteger bi => bi.ToByteArrayStandard(),
-                    IInteroperable interoperable => BinarySerializer.Serialize(interoperable.ToStackItem(null), 1024 * 1024),
+                    IInteroperable interoperable => BinarySerializer.Serialize(interoperable.ToStackItem(null), ExecutionEngineLimits.Default),
                     null => ReadOnlyMemory<byte>.Empty,
                     _ => throw new InvalidCastException()
                 };

tests/Neo.UnitTests/Ledger/UT_HashIndexState.cs

@@ -25,10 +25,10 @@ public void Initialize()
         [TestMethod]
         public void TestDeserialize()
         {
-            var data = BinarySerializer.Serialize(((IInteroperable)origin).ToStackItem(null), 1024);
+            var data = BinarySerializer.Serialize(((IInteroperable)origin).ToStackItem(null), ExecutionEngineLimits.Default);
             var reader = new MemoryReader(data);
 
-            HashIndexState dest = new HashIndexState();
+            HashIndexState dest = new();
             ((IInteroperable)dest).FromStackItem(BinarySerializer.Deserialize(ref reader, ExecutionEngineLimits.Default, null));
 
             dest.Hash.Should().Be(origin.Hash);

tests/Neo.UnitTests/Ledger/UT_TransactionState.cs

@@ -43,7 +43,7 @@ public void Initialize()
         [TestMethod]
         public void TestDeserialize()
         {
-            var data = BinarySerializer.Serialize(((IInteroperable)origin).ToStackItem(null), 1024);
+            var data = BinarySerializer.Serialize(((IInteroperable)origin).ToStackItem(null), ExecutionEngineLimits.Default);
             var reader = new MemoryReader(data);
 
             TransactionState dest = new();
@@ -57,7 +57,7 @@ public void TestDeserialize()
         [TestMethod]
         public void TestDeserializeTrimmed()
         {
-            var data = BinarySerializer.Serialize(((IInteroperable)originTrimmed).ToStackItem(null), 1024);
+            var data = BinarySerializer.Serialize(((IInteroperable)originTrimmed).ToStackItem(null), ExecutionEngineLimits.Default);
             var reader = new MemoryReader(data);
 
             TransactionState dest = new();

tests/Neo.UnitTests/SmartContract/Manifest/UT_ContractPermission.cs

@@ -2,13 +2,40 @@
 using Neo.Cryptography.ECC;
 using Neo.SmartContract;
 using Neo.SmartContract.Manifest;
+using Neo.VM.Types;
 using System;
 
 namespace Neo.UnitTests.SmartContract.Manifest
 {
     [TestClass]
     public class UT_ContractPermission
     {
+        [TestMethod]
+        public void TestDeserialize()
+        {
+            // null
+            ContractPermission contractPermission = ContractPermission.DefaultPermission;
+            Struct s = (Struct)contractPermission.ToStackItem(new VM.ReferenceCounter());
+
+            contractPermission = s.ToInteroperable<ContractPermission>();
+            Assert.IsTrue(contractPermission.Contract.IsWildcard);
+            Assert.IsTrue(contractPermission.Methods.IsWildcard);
+
+            // not null
+            contractPermission = new ContractPermission()
+            {
+                Contract = ContractPermissionDescriptor.Create(UInt160.Zero),
+                Methods = WildcardContainer<string>.Create("test")
+            };
+            s = (Struct)contractPermission.ToStackItem(new VM.ReferenceCounter());
+
+            contractPermission = s.ToInteroperable<ContractPermission>();
+            Assert.IsFalse(contractPermission.Contract.IsWildcard);
+            Assert.IsFalse(contractPermission.Methods.IsWildcard);
+            Assert.AreEqual(UInt160.Zero, contractPermission.Contract.Hash);
+            Assert.AreEqual("test", contractPermission.Methods[0]);
+        }
+
         [TestMethod]
         public void TestIsAllowed()
         {

tests/Neo.UnitTests/SmartContract/Native/UT_NeoToken.cs

@@ -771,7 +771,7 @@ public void TestClaimGas()
                 }));
                 cachedCommittee.Add((member, 200 * 10000));
             }
-            snapshot.GetOrAdd(new KeyBuilder(NativeContract.NEO.Id, 14), () => new StorageItem()).Value = BinarySerializer.Serialize(cachedCommittee.ToStackItem(null), 4096);
+            snapshot.GetOrAdd(new KeyBuilder(NativeContract.NEO.Id, 14), () => new StorageItem()).Value = BinarySerializer.Serialize(cachedCommittee.ToStackItem(null), ExecutionEngineLimits.Default);
 
             var item = snapshot.GetAndChange(new KeyBuilder(NativeContract.NEO.Id, 1), () => new StorageItem());
             item.Value = ((BigInteger)2100 * 10000L).ToByteArray();