From 2d0f9481a851cff14f4019f9a42b6b2b5d3a1deb Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Wed, 28 Feb 2024 07:50:45 +0100 Subject: [PATCH 1/4] Avoid load nefFile without some checksum --- src/Neo/SmartContract/ContractState.cs | 2 +- src/Neo/SmartContract/NefFile.cs | 25 ++++++++++++++++++++++--- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/Neo/SmartContract/ContractState.cs b/src/Neo/SmartContract/ContractState.cs index 83ac9d3353..e698d090f1 100644 --- a/src/Neo/SmartContract/ContractState.cs +++ b/src/Neo/SmartContract/ContractState.cs @@ -83,7 +83,7 @@ void IInteroperable.FromStackItem(StackItem stackItem) Id = (int)array[0].GetInteger(); UpdateCounter = (ushort)array[1].GetInteger(); Hash = new UInt160(array[2].GetSpan()); - Nef = ((ByteString)array[3]).Memory.AsSerializable(); + Nef = NefFile.Parse(((ByteString)array[3]).Memory, false); Manifest = array[4].ToInteroperable(); } diff --git a/src/Neo/SmartContract/NefFile.cs b/src/Neo/SmartContract/NefFile.cs index 1ffa7baf20..f8ad18a692 100644 --- a/src/Neo/SmartContract/NefFile.cs +++ b/src/Neo/SmartContract/NefFile.cs @@ -86,6 +86,20 @@ public class NefFile : ISerializable Script.GetVarSize() + // Script sizeof(uint); // Checksum + /// + /// Parse NefFile from memory + /// + /// Memory + /// Do checksum and MaxItemSize checks + /// NefFile + public static NefFile Parse(ReadOnlyMemory memory, bool ensureValid = true) + { + var reader = new MemoryReader(memory); + var nef = new NefFile(); + nef.Deserialize(ref reader, ensureValid); + return nef; + } + public void Serialize(BinaryWriter writer) { SerializeHeader(writer); @@ -103,7 +117,9 @@ private void SerializeHeader(BinaryWriter writer) writer.WriteFixedString(Compiler, 64); } - public void Deserialize(ref MemoryReader reader) + public void Deserialize(ref MemoryReader reader) => Deserialize(ref reader, true); + + public void Deserialize(ref MemoryReader reader, bool ensureValid) { long startPosition = reader.Position; if (reader.ReadUInt32() != Magic) throw new FormatException("Wrong magic"); @@ -115,8 +131,11 @@ public void Deserialize(ref MemoryReader reader) Script = reader.ReadVarMemory((int)ExecutionEngineLimits.Default.MaxItemSize); if (Script.Length == 0) throw new ArgumentException($"Script can't be empty"); CheckSum = reader.ReadUInt32(); - if (CheckSum != ComputeChecksum(this)) throw new FormatException("CRC verification fail"); - if (reader.Position - startPosition > ExecutionEngineLimits.Default.MaxItemSize) throw new FormatException("Max vm item size exceed"); + if (ensureValid) + { + if (CheckSum != ComputeChecksum(this)) throw new FormatException("CRC verification fail"); + if (reader.Position - startPosition > ExecutionEngineLimits.Default.MaxItemSize) throw new FormatException("Max vm item size exceed"); + } } /// From 99b0bf2658947e0972543c51405610820fab14b4 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Tue, 5 Mar 2024 09:50:22 +0100 Subject: [PATCH 2/4] Core's feedback --- src/Neo/SmartContract/ContractState.cs | 13 ++++++--- .../SmartContract/IInteroperableValidation.cs | 28 +++++++++++++++++++ .../Native/ContractManagement.cs | 10 +++---- src/Neo/SmartContract/StorageItem.cs | 18 ++++++++++++ 4 files changed, 60 insertions(+), 9 deletions(-) create mode 100644 src/Neo/SmartContract/IInteroperableValidation.cs diff --git a/src/Neo/SmartContract/ContractState.cs b/src/Neo/SmartContract/ContractState.cs index e698d090f1..d1f7fc2b73 100644 --- a/src/Neo/SmartContract/ContractState.cs +++ b/src/Neo/SmartContract/ContractState.cs @@ -23,7 +23,7 @@ namespace Neo.SmartContract /// /// Represents a deployed contract. /// - public class ContractState : IInteroperable + public class ContractState : IInteroperableValidation { /// /// The id of the contract. @@ -69,7 +69,7 @@ IInteroperable IInteroperable.Clone() void IInteroperable.FromReplica(IInteroperable replica) { - ContractState from = (ContractState)replica; + var from = (ContractState)replica; Id = from.Id; UpdateCounter = from.UpdateCounter; Hash = from.Hash; @@ -79,11 +79,16 @@ void IInteroperable.FromReplica(IInteroperable replica) void IInteroperable.FromStackItem(StackItem stackItem) { - Array array = (Array)stackItem; + ((IInteroperableValidation)this).FromStackItem(stackItem, true); + } + + void IInteroperableValidation.FromStackItem(StackItem stackItem, bool validate) + { + var array = (Array)stackItem; Id = (int)array[0].GetInteger(); UpdateCounter = (ushort)array[1].GetInteger(); Hash = new UInt160(array[2].GetSpan()); - Nef = NefFile.Parse(((ByteString)array[3]).Memory, false); + Nef = NefFile.Parse(((ByteString)array[3]).Memory, validate); Manifest = array[4].ToInteroperable(); } diff --git a/src/Neo/SmartContract/IInteroperableValidation.cs b/src/Neo/SmartContract/IInteroperableValidation.cs new file mode 100644 index 0000000000..e835430f40 --- /dev/null +++ b/src/Neo/SmartContract/IInteroperableValidation.cs @@ -0,0 +1,28 @@ +// Copyright (C) 2015-2024 The Neo Project. +// +// IInteroperable.cs file belongs to the neo project and is free +// software distributed under the MIT software license, see the +// accompanying file LICENSE in the main directory of the +// repository or http://www.opensource.org/licenses/mit-license.php +// for more details. +// +// Redistribution and use in source and binary forms with or without +// modifications are permitted. + +using Neo.VM.Types; + +namespace Neo.SmartContract +{ + /// + /// Represents the object that can be converted to and from . + /// + public interface IInteroperableValidation : IInteroperable + { + /// + /// Convert a to the current object. + /// + /// The to convert. + /// Validate + void FromStackItem(StackItem stackItem, bool validate); + } +} diff --git a/src/Neo/SmartContract/Native/ContractManagement.cs b/src/Neo/SmartContract/Native/ContractManagement.cs index 3347163a6c..28642a2db1 100644 --- a/src/Neo/SmartContract/Native/ContractManagement.cs +++ b/src/Neo/SmartContract/Native/ContractManagement.cs @@ -84,7 +84,7 @@ internal override async ContractTask OnPersist(ApplicationEngine engine) else { // Parse old contract - var oldContract = state.GetInteroperable(); + var oldContract = state.GetInteroperable(false); // Increase the update counter oldContract.UpdateCounter++; // Modify nef and manifest @@ -122,7 +122,7 @@ private void SetMinimumDeploymentFee(ApplicationEngine engine, BigInteger value) [ContractMethod(CpuFee = 1 << 15, RequiredCallFlags = CallFlags.ReadStates)] public ContractState GetContract(DataCache snapshot, UInt160 hash) { - return snapshot.TryGet(CreateStorageKey(Prefix_Contract).Add(hash))?.GetInteroperable(); + return snapshot.TryGet(CreateStorageKey(Prefix_Contract).Add(hash))?.GetInteroperable(false); } /// @@ -183,7 +183,7 @@ public bool HasMethod(DataCache snapshot, UInt160 hash, string method, int pcoun public IEnumerable ListContracts(DataCache snapshot) { byte[] listContractsPrefix = CreateStorageKey(Prefix_Contract).ToArray(); - return snapshot.Find(listContractsPrefix).Select(kvp => kvp.Value.GetInteroperable()); + return snapshot.Find(listContractsPrefix).Select(kvp => kvp.Value.GetInteroperable(false)); } [ContractMethod(RequiredCallFlags = CallFlags.All)] @@ -250,7 +250,7 @@ private ContractTask Update(ApplicationEngine engine, byte[] nefFile, byte[] man engine.AddGas(engine.StoragePrice * ((nefFile?.Length ?? 0) + (manifest?.Length ?? 0))); - var contract = engine.Snapshot.GetAndChange(CreateStorageKey(Prefix_Contract).Add(engine.CallingScriptHash))?.GetInteroperable(); + var contract = engine.Snapshot.GetAndChange(CreateStorageKey(Prefix_Contract).Add(engine.CallingScriptHash))?.GetInteroperable(false); if (contract is null) throw new InvalidOperationException($"Updating Contract Does Not Exist: {engine.CallingScriptHash}"); if (contract.UpdateCounter == ushort.MaxValue) throw new InvalidOperationException($"The contract reached the maximum number of updates."); @@ -283,7 +283,7 @@ private void Destroy(ApplicationEngine engine) { UInt160 hash = engine.CallingScriptHash; StorageKey ckey = CreateStorageKey(Prefix_Contract).Add(hash); - ContractState contract = engine.Snapshot.TryGet(ckey)?.GetInteroperable(); + ContractState contract = engine.Snapshot.TryGet(ckey)?.GetInteroperable(false); if (contract is null) return; engine.Snapshot.Delete(ckey); engine.Snapshot.Delete(CreateStorageKey(Prefix_ContractHash).AddBigEndian(contract.Id)); diff --git a/src/Neo/SmartContract/StorageItem.cs b/src/Neo/SmartContract/StorageItem.cs index 41486997de..8c43d2959e 100644 --- a/src/Neo/SmartContract/StorageItem.cs +++ b/src/Neo/SmartContract/StorageItem.cs @@ -145,6 +145,24 @@ public void FromReplica(StorageItem replica) return (T)cache; } + /// + /// Gets an from the storage. + /// + /// Validate deserialization + /// The type of the . + /// The in the storage. + public T GetInteroperable(bool validate) where T : IInteroperableValidation, new() + { + if (cache is null) + { + var interoperable = new T(); + interoperable.FromStackItem(BinarySerializer.Deserialize(value, ExecutionEngineLimits.Default), validate); + cache = interoperable; + } + value = null; + return (T)cache; + } + public void Serialize(BinaryWriter writer) { writer.Write(Value.Span); From a03cb6c79adcacc2f5cc397bebad8dac41d003d2 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Wed, 6 Mar 2024 09:00:50 +0100 Subject: [PATCH 3/4] Anna's feedback --- src/Neo/SmartContract/ContractState.cs | 8 ++++---- ...rableValidation.cs => IInteroperableVerifiable.cs} | 11 ++++++----- src/Neo/SmartContract/StorageItem.cs | 6 +++--- 3 files changed, 13 insertions(+), 12 deletions(-) rename src/Neo/SmartContract/{IInteroperableValidation.cs => IInteroperableVerifiable.cs} (66%) diff --git a/src/Neo/SmartContract/ContractState.cs b/src/Neo/SmartContract/ContractState.cs index d1f7fc2b73..5b413c41a4 100644 --- a/src/Neo/SmartContract/ContractState.cs +++ b/src/Neo/SmartContract/ContractState.cs @@ -23,7 +23,7 @@ namespace Neo.SmartContract /// /// Represents a deployed contract. /// - public class ContractState : IInteroperableValidation + public class ContractState : IInteroperableVerifiable { /// /// The id of the contract. @@ -79,16 +79,16 @@ void IInteroperable.FromReplica(IInteroperable replica) void IInteroperable.FromStackItem(StackItem stackItem) { - ((IInteroperableValidation)this).FromStackItem(stackItem, true); + ((IInteroperableVerifiable)this).FromStackItem(stackItem, true); } - void IInteroperableValidation.FromStackItem(StackItem stackItem, bool validate) + void IInteroperableVerifiable.FromStackItem(StackItem stackItem, bool verify) { var array = (Array)stackItem; Id = (int)array[0].GetInteger(); UpdateCounter = (ushort)array[1].GetInteger(); Hash = new UInt160(array[2].GetSpan()); - Nef = NefFile.Parse(((ByteString)array[3]).Memory, validate); + Nef = NefFile.Parse(((ByteString)array[3]).Memory, verify); Manifest = array[4].ToInteroperable(); } diff --git a/src/Neo/SmartContract/IInteroperableValidation.cs b/src/Neo/SmartContract/IInteroperableVerifiable.cs similarity index 66% rename from src/Neo/SmartContract/IInteroperableValidation.cs rename to src/Neo/SmartContract/IInteroperableVerifiable.cs index e835430f40..c8af7b8a5e 100644 --- a/src/Neo/SmartContract/IInteroperableValidation.cs +++ b/src/Neo/SmartContract/IInteroperableVerifiable.cs @@ -1,6 +1,6 @@ // Copyright (C) 2015-2024 The Neo Project. // -// IInteroperable.cs file belongs to the neo project and is free +// IInteroperableVerifiable.cs file belongs to the neo project and is free // software distributed under the MIT software license, see the // accompanying file LICENSE in the main directory of the // repository or http://www.opensource.org/licenses/mit-license.php @@ -14,15 +14,16 @@ namespace Neo.SmartContract { /// - /// Represents the object that can be converted to and from . + /// Represents the object that can be converted to and from + /// and allows you to specify whether a verification is required. /// - public interface IInteroperableValidation : IInteroperable + public interface IInteroperableVerifiable : IInteroperable { /// /// Convert a to the current object. /// /// The to convert. - /// Validate - void FromStackItem(StackItem stackItem, bool validate); + /// Verify the content + void FromStackItem(StackItem stackItem, bool verify = true); } } diff --git a/src/Neo/SmartContract/StorageItem.cs b/src/Neo/SmartContract/StorageItem.cs index 8c43d2959e..350e95e70a 100644 --- a/src/Neo/SmartContract/StorageItem.cs +++ b/src/Neo/SmartContract/StorageItem.cs @@ -148,15 +148,15 @@ public void FromReplica(StorageItem replica) /// /// Gets an from the storage. /// - /// Validate deserialization + /// Verify deserialization /// The type of the . /// The in the storage. - public T GetInteroperable(bool validate) where T : IInteroperableValidation, new() + public T GetInteroperable(bool verify = true) where T : IInteroperableVerifiable, new() { if (cache is null) { var interoperable = new T(); - interoperable.FromStackItem(BinarySerializer.Deserialize(value, ExecutionEngineLimits.Default), validate); + interoperable.FromStackItem(BinarySerializer.Deserialize(value, ExecutionEngineLimits.Default), verify); cache = interoperable; } value = null; From 9ba6df005c5f2edd7a2f8acf8b87e9c4822fde24 Mon Sep 17 00:00:00 2001 From: Fernando Diaz Toledano Date: Wed, 6 Mar 2024 09:02:13 +0100 Subject: [PATCH 4/4] Add defaults --- src/Neo/SmartContract/NefFile.cs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/Neo/SmartContract/NefFile.cs b/src/Neo/SmartContract/NefFile.cs index f8ad18a692..3e343d3be2 100644 --- a/src/Neo/SmartContract/NefFile.cs +++ b/src/Neo/SmartContract/NefFile.cs @@ -90,13 +90,13 @@ public class NefFile : ISerializable /// Parse NefFile from memory /// /// Memory - /// Do checksum and MaxItemSize checks + /// Do checksum and MaxItemSize checks /// NefFile - public static NefFile Parse(ReadOnlyMemory memory, bool ensureValid = true) + public static NefFile Parse(ReadOnlyMemory memory, bool verify = true) { var reader = new MemoryReader(memory); var nef = new NefFile(); - nef.Deserialize(ref reader, ensureValid); + nef.Deserialize(ref reader, verify); return nef; } @@ -119,7 +119,7 @@ private void SerializeHeader(BinaryWriter writer) public void Deserialize(ref MemoryReader reader) => Deserialize(ref reader, true); - public void Deserialize(ref MemoryReader reader, bool ensureValid) + public void Deserialize(ref MemoryReader reader, bool verify = true) { long startPosition = reader.Position; if (reader.ReadUInt32() != Magic) throw new FormatException("Wrong magic"); @@ -131,7 +131,7 @@ public void Deserialize(ref MemoryReader reader, bool ensureValid) Script = reader.ReadVarMemory((int)ExecutionEngineLimits.Default.MaxItemSize); if (Script.Length == 0) throw new ArgumentException($"Script can't be empty"); CheckSum = reader.ReadUInt32(); - if (ensureValid) + if (verify) { if (CheckSum != ComputeChecksum(this)) throw new FormatException("CRC verification fail"); if (reader.Position - startPosition > ExecutionEngineLimits.Default.MaxItemSize) throw new FormatException("Max vm item size exceed");