From c043a2b1c97fbadd11e4ee4a05638fe3750fafc2 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 14:15:32 +0800 Subject: [PATCH 01/15] Check script and abi when deploying contracts --- .../Manifest/ContractManifest.cs | 8 +- .../Manifest/ContractMethodDescriptor.cs | 4 +- .../Manifest/ContractParameterDefinition.cs | 5 +- .../Native/ContractManagement.cs | 81 ++++++++++++++++++- .../SmartContract/UT_InteropService.NEO.cs | 7 +- 5 files changed, 96 insertions(+), 9 deletions(-) diff --git a/src/neo/SmartContract/Manifest/ContractManifest.cs b/src/neo/SmartContract/Manifest/ContractManifest.cs index f356639a1d..44841b81bd 100644 --- a/src/neo/SmartContract/Manifest/ContractManifest.cs +++ b/src/neo/SmartContract/Manifest/ContractManifest.cs @@ -110,9 +110,13 @@ public static ContractManifest FromJson(JObject json) /// /// Json /// Return ContractManifest - public static ContractManifest Parse(ReadOnlySpan json) => FromJson(JObject.Parse(json)); + public static ContractManifest Parse(ReadOnlySpan json) + { + if (json.Length > MaxLength) throw new ArgumentException(null, nameof(json)); + return FromJson(JObject.Parse(json)); + } - public static ContractManifest Parse(string json) => FromJson(JObject.Parse(json)); + public static ContractManifest Parse(string json) => Parse(Utility.StrictUTF8.GetBytes(json)); /// Return ContractMethodDescription public new static ContractMethodDescriptor FromJson(JObject json) { + ContractParameterType returnType = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["returntype"].AsString()); + if (!Enum.IsDefined(returnType)) throw new FormatException(); return new ContractMethodDescriptor { Name = json["name"].AsString(), Parameters = ((JArray)json["parameters"]).Select(u => ContractParameterDefinition.FromJson(u)).ToArray(), - ReturnType = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["returntype"].AsString()), + ReturnType = returnType, Offset = (int)json["offset"].AsNumber(), Safe = json["safe"].AsBoolean(), }; diff --git a/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs b/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs index 71f4f2391f..9f8910973b 100644 --- a/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs +++ b/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs @@ -37,10 +37,13 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractParameterDefinition public static ContractParameterDefinition FromJson(JObject json) { + ContractParameterType type = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["type"].AsString()); + if (!Enum.IsDefined(type) || type == ContractParameterType.Void) + throw new FormatException(); return new ContractParameterDefinition { Name = json["name"].AsString(), - Type = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["type"].AsString()), + Type = type, }; } diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 2bf9bd9231..c39b40f679 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -4,6 +4,7 @@ using Neo.Network.P2P.Payloads; using Neo.Persistence; using Neo.SmartContract.Manifest; +using Neo.VM; using Neo.VM.Types; using System; using System.Collections.Generic; @@ -63,6 +64,80 @@ internal ContractManagement() Manifest.Abi.Events = events.ToArray(); } + private static void CheckScriptAndAbi(Script script, ContractAbi abi) + { + Dictionary instructions = new Dictionary(); + for (int ip = 0; ip < script.Length;) + { + Instruction instruction = script.GetInstruction(ip); + instructions.Add(ip, instruction); + ip += instruction.Size; + } + foreach (var (ip, instruction) in instructions) + { + switch (instruction.OpCode) + { + case OpCode.JMP: + case OpCode.JMPIF: + case OpCode.JMPIFNOT: + case OpCode.JMPEQ: + case OpCode.JMPNE: + case OpCode.JMPGT: + case OpCode.JMPGE: + case OpCode.JMPLT: + case OpCode.JMPLE: + case OpCode.CALL: + case OpCode.ENDTRY: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) + throw new ArgumentException(null, nameof(script)); + break; + case OpCode.PUSHA: + case OpCode.JMP_L: + case OpCode.JMPIF_L: + case OpCode.JMPIFNOT_L: + case OpCode.JMPEQ_L: + case OpCode.JMPNE_L: + case OpCode.JMPGT_L: + case OpCode.JMPGE_L: + case OpCode.JMPLT_L: + case OpCode.JMPLE_L: + case OpCode.CALL_L: + case OpCode.ENDTRY_L: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) + throw new ArgumentException(null, nameof(script)); + break; + case OpCode.TRY: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) + throw new ArgumentException(null, nameof(script)); + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8_1))) + throw new ArgumentException(null, nameof(script)); + break; + case OpCode.TRY_L: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) + throw new ArgumentException(null, nameof(script)); + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32_1))) + throw new ArgumentException(null, nameof(script)); + break; + case OpCode.NEWARRAY_T: + case OpCode.ISTYPE: + case OpCode.CONVERT: + StackItemType type = (StackItemType)instruction.TokenU8; + if (!Enum.IsDefined(typeof(StackItemType), type)) + throw new ArgumentException(null, nameof(script)); + if (instruction.OpCode != OpCode.NEWARRAY_T && type == StackItemType.Any) + throw new ArgumentException(null, nameof(script)); + break; + } + } + foreach (ContractMethodDescriptor method in abi.Methods) + { + if (!instructions.ContainsKey(method.Offset)) + throw new ArgumentException(null, nameof(script)); + abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. + } + _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. + } + private int GetNextAvailableId(DataCache snapshot) { StorageItem item = snapshot.GetAndChange(CreateStorageKey(Prefix_NextAvailableId), () => new StorageItem(1)); @@ -132,7 +207,7 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma throw new InvalidOperationException(); if (nefFile.Length == 0) throw new ArgumentException($"Invalid NefFile Length: {nefFile.Length}"); - if (manifest.Length == 0 || manifest.Length > ContractManifest.MaxLength) + if (manifest.Length == 0) throw new ArgumentException($"Invalid Manifest Length: {manifest.Length}"); engine.AddGas(Math.Max( @@ -142,6 +217,7 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma NefFile nef = nefFile.AsSerializable(); ContractManifest parsedManifest = ContractManifest.Parse(manifest); + CheckScriptAndAbi(nef.Script, parsedManifest.Abi); UInt160 hash = Helper.GetContractHash(tx.Sender, nef.CheckSum, parsedManifest.Name); StorageKey key = CreateStorageKey(Prefix_Contract).Add(hash); if (engine.Snapshot.Contains(key)) @@ -196,7 +272,7 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S } if (manifest != null) { - if (manifest.Length == 0 || manifest.Length > ContractManifest.MaxLength) + if (manifest.Length == 0) throw new ArgumentException($"Invalid Manifest Length: {manifest.Length}"); ContractManifest manifest_new = ContractManifest.Parse(manifest); if (manifest_new.Name != contract.Manifest.Name) @@ -205,6 +281,7 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}"); contract.Manifest = manifest_new; } + CheckScriptAndAbi(contract.Nef.Script, contract.Manifest.Abi); contract.UpdateCounter++; // Increase update counter if (nefFile != null) { diff --git a/tests/neo.UnitTests/SmartContract/UT_InteropService.NEO.cs b/tests/neo.UnitTests/SmartContract/UT_InteropService.NEO.cs index d721c3936a..32c48095a2 100644 --- a/tests/neo.UnitTests/SmartContract/UT_InteropService.NEO.cs +++ b/tests/neo.UnitTests/SmartContract/UT_InteropService.NEO.cs @@ -11,6 +11,7 @@ using Neo.SmartContract.Manifest; using Neo.SmartContract.Native; using Neo.UnitTests.Extensions; +using Neo.VM; using Neo.VM.Types; using Neo.Wallets; using System; @@ -133,7 +134,7 @@ public void TestContract_Create() var snapshot = Blockchain.Singleton.GetSnapshot().CreateSnapshot(); var nef = new NefFile() { - Script = new byte[byte.MaxValue], + Script = Enumerable.Repeat((byte)OpCode.RET, byte.MaxValue).ToArray(), Compiler = "", Tokens = System.Array.Empty() }; @@ -161,7 +162,7 @@ public void TestContract_Create() manifest = TestUtils.CreateDefaultManifest(); var ret = snapshot.DeployContract(UInt160.Zero, nefFile, manifest.ToJson().ToByteArray(false)); - ret.Hash.ToString().Should().Be("0x18e26e71e66fb79347581771c0910df5bea9d24b"); + ret.Hash.ToString().Should().Be("0x7b37d4bd3d87f53825c3554bd1a617318235a685"); Assert.ThrowsException(() => snapshot.DeployContract(UInt160.Zero, nefFile, manifest.ToJson().ToByteArray(false))); var state = TestUtils.GetContract(); @@ -176,7 +177,7 @@ public void TestContract_Update() var snapshot = Blockchain.Singleton.GetSnapshot().CreateSnapshot(); var nef = new NefFile() { - Script = new byte[] { 0x01 }, + Script = new[] { (byte)OpCode.RET }, Compiler = "", Tokens = System.Array.Empty() }; From 94a570dc005f9c318092204cccf386cb67b2425b Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 17:24:48 +0800 Subject: [PATCH 02/15] Shargon's suggestion --- src/neo/SmartContract/Native/ContractManagement.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index c39b40f679..74b28cd09b 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -133,8 +133,8 @@ private static void CheckScriptAndAbi(Script script, ContractAbi abi) { if (!instructions.ContainsKey(method.Offset)) throw new ArgumentException(null, nameof(script)); - abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. } + abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. } From abcbe4c5c81e26ebc0c8e193e1ea5a24a2df0e7b Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 21:43:04 +0800 Subject: [PATCH 03/15] More checks --- src/neo/SmartContract/Manifest/ContractAbi.cs | 4 +++- .../Manifest/ContractEventDescriptor.cs | 7 +++++- .../SmartContract/Manifest/ContractGroup.cs | 4 +++- .../Manifest/ContractManifest.cs | 11 +++++++++- .../Manifest/ContractMethodDescriptor.cs | 18 +++++++-------- .../Manifest/ContractParameterDefinition.cs | 12 +++++----- .../Manifest/ContractPermission.cs | 6 ++++- .../Manifest/ContractPermissionDescriptor.cs | 22 ++++++++++++++++++- .../Manifest/UT_ContractManifest.cs | 12 +++++----- tests/neo.UnitTests/TestUtils.cs | 13 +++++++++-- 10 files changed, 80 insertions(+), 29 deletions(-) diff --git a/src/neo/SmartContract/Manifest/ContractAbi.cs b/src/neo/SmartContract/Manifest/ContractAbi.cs index b2d890b111..db4fdfe344 100644 --- a/src/neo/SmartContract/Manifest/ContractAbi.cs +++ b/src/neo/SmartContract/Manifest/ContractAbi.cs @@ -48,11 +48,13 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractAbi public static ContractAbi FromJson(JObject json) { - return new ContractAbi + ContractAbi abi = new ContractAbi { Methods = ((JArray)json["methods"]).Select(u => ContractMethodDescriptor.FromJson(u)).ToArray(), Events = ((JArray)json["events"]).Select(u => ContractEventDescriptor.FromJson(u)).ToArray() }; + if (abi.Methods.Length == 0) throw new FormatException(); + return abi; } public ContractMethodDescriptor GetMethod(string name, int pcount) diff --git a/src/neo/SmartContract/Manifest/ContractEventDescriptor.cs b/src/neo/SmartContract/Manifest/ContractEventDescriptor.cs index ea5787332c..7d8afd1121 100644 --- a/src/neo/SmartContract/Manifest/ContractEventDescriptor.cs +++ b/src/neo/SmartContract/Manifest/ContractEventDescriptor.cs @@ -1,7 +1,9 @@ using Neo.IO.Json; using Neo.VM; using Neo.VM.Types; +using System; using System.Linq; +using Array = Neo.VM.Types.Array; namespace Neo.SmartContract.Manifest { @@ -40,11 +42,14 @@ public virtual StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractEventDescriptor public static ContractEventDescriptor FromJson(JObject json) { - return new ContractEventDescriptor + ContractEventDescriptor descriptor = new ContractEventDescriptor { Name = json["name"].AsString(), Parameters = ((JArray)json["parameters"]).Select(u => ContractParameterDefinition.FromJson(u)).ToArray(), }; + if (string.IsNullOrEmpty(descriptor.Name)) throw new FormatException(); + _ = descriptor.Parameters.ToDictionary(p => p.Name); + return descriptor; } public virtual JObject ToJson() diff --git a/src/neo/SmartContract/Manifest/ContractGroup.cs b/src/neo/SmartContract/Manifest/ContractGroup.cs index 9d1caa5250..b39c44c0dd 100644 --- a/src/neo/SmartContract/Manifest/ContractGroup.cs +++ b/src/neo/SmartContract/Manifest/ContractGroup.cs @@ -43,11 +43,13 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractManifestGroup public static ContractGroup FromJson(JObject json) { - return new ContractGroup + ContractGroup group = new ContractGroup { PubKey = ECPoint.Parse(json["pubkey"].AsString(), ECCurve.Secp256r1), Signature = Convert.FromBase64String(json["signature"].AsString()), }; + if (group.Signature.Length != 64) throw new FormatException(); + return group; } /// diff --git a/src/neo/SmartContract/Manifest/ContractManifest.cs b/src/neo/SmartContract/Manifest/ContractManifest.cs index 44841b81bd..71d93020f2 100644 --- a/src/neo/SmartContract/Manifest/ContractManifest.cs +++ b/src/neo/SmartContract/Manifest/ContractManifest.cs @@ -93,7 +93,7 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractManifest public static ContractManifest FromJson(JObject json) { - return new ContractManifest + ContractManifest manifest = new ContractManifest { Name = json["name"].AsString(), Groups = ((JArray)json["groups"]).Select(u => ContractGroup.FromJson(u)).ToArray(), @@ -103,6 +103,15 @@ public static ContractManifest FromJson(JObject json) Trusts = WildcardContainer.FromJson(json["trusts"], u => UInt160.Parse(u.AsString())), Extra = json["extra"] }; + if (string.IsNullOrEmpty(manifest.Name)) + throw new FormatException(); + _ = manifest.Groups.ToDictionary(p => p.PubKey); + if (manifest.SupportedStandards.Any(p => string.IsNullOrEmpty(p))) + throw new FormatException(); + _ = manifest.SupportedStandards.ToDictionary(p => p); + _ = manifest.Permissions.ToDictionary(p => p.Contract); + _ = manifest.Trusts.ToDictionary(p => p); + return manifest; } /// diff --git a/src/neo/SmartContract/Manifest/ContractMethodDescriptor.cs b/src/neo/SmartContract/Manifest/ContractMethodDescriptor.cs index 3974107e0b..b9374d98e2 100644 --- a/src/neo/SmartContract/Manifest/ContractMethodDescriptor.cs +++ b/src/neo/SmartContract/Manifest/ContractMethodDescriptor.cs @@ -14,12 +14,7 @@ public class ContractMethodDescriptor : ContractEventDescriptor /// public ContractParameterType ReturnType { get; set; } - private int _offset; - public int Offset - { - get => _offset; - set => _offset = value >= 0 ? value : throw new FormatException(); - } + public int Offset { get; set; } /// /// Determine if it's safe to call this method @@ -52,16 +47,19 @@ public override StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractMethodDescription public new static ContractMethodDescriptor FromJson(JObject json) { - ContractParameterType returnType = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["returntype"].AsString()); - if (!Enum.IsDefined(returnType)) throw new FormatException(); - return new ContractMethodDescriptor + ContractMethodDescriptor descriptor = new ContractMethodDescriptor { Name = json["name"].AsString(), Parameters = ((JArray)json["parameters"]).Select(u => ContractParameterDefinition.FromJson(u)).ToArray(), - ReturnType = returnType, + ReturnType = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["returntype"].AsString()), Offset = (int)json["offset"].AsNumber(), Safe = json["safe"].AsBoolean(), }; + if (string.IsNullOrEmpty(descriptor.Name)) throw new FormatException(); + _ = descriptor.Parameters.ToDictionary(p => p.Name); + if (!Enum.IsDefined(descriptor.ReturnType)) throw new FormatException(); + if (descriptor.Offset < 0) throw new FormatException(); + return descriptor; } public override JObject ToJson() diff --git a/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs b/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs index 9f8910973b..4bfed05262 100644 --- a/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs +++ b/src/neo/SmartContract/Manifest/ContractParameterDefinition.cs @@ -37,14 +37,16 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractParameterDefinition public static ContractParameterDefinition FromJson(JObject json) { - ContractParameterType type = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["type"].AsString()); - if (!Enum.IsDefined(type) || type == ContractParameterType.Void) - throw new FormatException(); - return new ContractParameterDefinition + ContractParameterDefinition parameter = new ContractParameterDefinition { Name = json["name"].AsString(), - Type = type, + Type = (ContractParameterType)Enum.Parse(typeof(ContractParameterType), json["type"].AsString()), }; + if (string.IsNullOrEmpty(parameter.Name)) + throw new FormatException(); + if (!Enum.IsDefined(parameter.Type) || parameter.Type == ContractParameterType.Void) + throw new FormatException(); + return parameter; } public virtual JObject ToJson() diff --git a/src/neo/SmartContract/Manifest/ContractPermission.cs b/src/neo/SmartContract/Manifest/ContractPermission.cs index 37658df284..14c2608349 100644 --- a/src/neo/SmartContract/Manifest/ContractPermission.cs +++ b/src/neo/SmartContract/Manifest/ContractPermission.cs @@ -63,11 +63,15 @@ public StackItem ToStackItem(ReferenceCounter referenceCounter) /// Return ContractPermission public static ContractPermission FromJson(JObject json) { - return new ContractPermission + ContractPermission permission = new ContractPermission { Contract = ContractPermissionDescriptor.FromJson(json["contract"]), Methods = WildcardContainer.FromJson(json["methods"], u => u.AsString()), }; + if (permission.Methods.Any(p => string.IsNullOrEmpty(p))) + throw new FormatException(); + _ = permission.Methods.ToDictionary(p => p); + return permission; } /// { public UInt160 Hash { get; } public ECPoint Group { get; } @@ -50,6 +50,26 @@ public static ContractPermissionDescriptor CreateWildcard() return new ContractPermissionDescriptor(null, null); } + public override bool Equals(object obj) + { + if (obj is not ContractPermissionDescriptor other) return false; + return Equals(other); + } + + public bool Equals(ContractPermissionDescriptor other) + { + if (other is null) return false; + if (this == other) return true; + if (IsWildcard == other.IsWildcard) return true; + if (IsHash) return Hash.Equals(other.Hash); + else return Group.Equals(other.Group); + } + + public override int GetHashCode() + { + return HashCode.Combine(Hash, Group); + } + public static ContractPermissionDescriptor FromJson(JObject json) { string str = json.AsString(); diff --git a/tests/neo.UnitTests/SmartContract/Manifest/UT_ContractManifest.cs b/tests/neo.UnitTests/SmartContract/Manifest/UT_ContractManifest.cs index df448e2e8a..d9a0d4950c 100644 --- a/tests/neo.UnitTests/SmartContract/Manifest/UT_ContractManifest.cs +++ b/tests/neo.UnitTests/SmartContract/Manifest/UT_ContractManifest.cs @@ -12,7 +12,7 @@ public class UT_ContractManifest [TestMethod] public void ParseFromJson_Default() { - var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; + var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(manifest.ToJson().ToString(), json); @@ -23,7 +23,7 @@ public void ParseFromJson_Default() [TestMethod] public void ParseFromJson_Permissions() { - var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[],""events"":[]},""permissions"":[{""contract"":""0x0000000000000000000000000000000000000000"",""methods"":[""method1"",""method2""]}],""trusts"":[],""extra"":null}"; + var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""0x0000000000000000000000000000000000000000"",""methods"":[""method1"",""method2""]}],""trusts"":[],""extra"":null}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(manifest.ToJson().ToString(), json); @@ -42,7 +42,7 @@ public void ParseFromJson_Permissions() [TestMethod] public void ParseFromJson_SafeMethods() { - var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; + var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(manifest.ToJson().ToString(), json); @@ -53,7 +53,7 @@ public void ParseFromJson_SafeMethods() [TestMethod] public void ParseFromJson_Trust() { - var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[""0x0000000000000000000000000000000000000001""],""extra"":null}"; + var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[""0x0000000000000000000000000000000000000001""],""extra"":null}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(manifest.ToJson().ToString(), json); @@ -65,7 +65,7 @@ public void ParseFromJson_Trust() [TestMethod] public void ParseFromJson_Groups() { - var json = @"{""name"":""testManifest"",""groups"":[{""pubkey"":""03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c"",""signature"":""QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQQ==""}],""supportedstandards"":[],""abi"":{""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; + var json = @"{""name"":""testManifest"",""groups"":[{""pubkey"":""03b209fd4f53a7170ea4444e0cb0a6bb6a53c2bd016926989cf85f9b0fba17a70c"",""signature"":""QUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQUFBQQ==""}],""supportedstandards"":[],""abi"":{""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":null}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(manifest.ToJson().ToString(), json); @@ -77,7 +77,7 @@ public void ParseFromJson_Groups() [TestMethod] public void ParseFromJson_Extra() { - var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""hash"":""0x0000000000000000000000000000000000000000"",""methods"":[],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":{""key"":""value""}}"; + var json = @"{""name"":""testManifest"",""groups"":[],""supportedstandards"":[],""abi"":{""hash"":""0x0000000000000000000000000000000000000000"",""methods"":[{""name"":""testMethod"",""parameters"":[],""returntype"":""Void"",""offset"":0,""safe"":true}],""events"":[]},""permissions"":[{""contract"":""*"",""methods"":""*""}],""trusts"":[],""extra"":{""key"":""value""}}"; var manifest = ContractManifest.Parse(json); Assert.AreEqual(json, json); Assert.AreEqual("value", manifest.Extra["key"].AsString(), false); diff --git a/tests/neo.UnitTests/TestUtils.cs b/tests/neo.UnitTests/TestUtils.cs index 76646ef3f9..f68e058d01 100644 --- a/tests/neo.UnitTests/TestUtils.cs +++ b/tests/neo.UnitTests/TestUtils.cs @@ -1,7 +1,6 @@ using FluentAssertions; using Neo.IO; using Neo.IO.Json; -using Neo.Ledger; using Neo.Network.P2P.Payloads; using Neo.SmartContract; using Neo.SmartContract.Manifest; @@ -28,7 +27,17 @@ public static ContractManifest CreateDefaultManifest() Abi = new ContractAbi() { Events = new ContractEventDescriptor[0], - Methods = new ContractMethodDescriptor[0] + Methods = new[] + { + new ContractMethodDescriptor + { + Name = "testMethod", + Parameters = new ContractParameterDefinition[0], + ReturnType = ContractParameterType.Void, + Offset = 0, + Safe = true + } + } }, Permissions = new[] { ContractPermission.DefaultPermission }, Trusts = WildcardContainer.Create(), From 35176ea80ad28f87f915ead1cb949e0d1f1e1aa2 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 23:50:59 +0800 Subject: [PATCH 04/15] Move method to Helper.cs --- src/neo/SmartContract/Helper.cs | 84 +++++++++++++++++++ .../Native/ContractManagement.cs | 79 +---------------- 2 files changed, 86 insertions(+), 77 deletions(-) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index bf8a6eae75..71dbb2baf3 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -9,6 +9,7 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; +using System.Linq; namespace Neo.SmartContract { @@ -29,6 +30,89 @@ public static long MultiSignatureContractCost(int m, int n) => ApplicationEngine.OpCodePrices[OpCode.SYSCALL] + ApplicationEngine.ECDsaVerifyPrice * n; + public static bool Check(Script script, ContractAbi abi = null) + { + Dictionary instructions = new Dictionary(); + for (int ip = 0; ip < script.Length;) + { + Instruction instruction = script.GetInstruction(ip); + instructions.Add(ip, instruction); + ip += instruction.Size; + } + foreach (var (ip, instruction) in instructions) + { + switch (instruction.OpCode) + { + case OpCode.JMP: + case OpCode.JMPIF: + case OpCode.JMPIFNOT: + case OpCode.JMPEQ: + case OpCode.JMPNE: + case OpCode.JMPGT: + case OpCode.JMPGE: + case OpCode.JMPLT: + case OpCode.JMPLE: + case OpCode.CALL: + case OpCode.ENDTRY: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) + return false; + break; + case OpCode.PUSHA: + case OpCode.JMP_L: + case OpCode.JMPIF_L: + case OpCode.JMPIFNOT_L: + case OpCode.JMPEQ_L: + case OpCode.JMPNE_L: + case OpCode.JMPGT_L: + case OpCode.JMPGE_L: + case OpCode.JMPLT_L: + case OpCode.JMPLE_L: + case OpCode.CALL_L: + case OpCode.ENDTRY_L: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) + return false; + break; + case OpCode.TRY: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) + return false; + if (!instructions.ContainsKey(checked(ip + instruction.TokenI8_1))) + return false; + break; + case OpCode.TRY_L: + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) + return false; + if (!instructions.ContainsKey(checked(ip + instruction.TokenI32_1))) + return false; + break; + case OpCode.NEWARRAY_T: + case OpCode.ISTYPE: + case OpCode.CONVERT: + StackItemType type = (StackItemType)instruction.TokenU8; + if (!Enum.IsDefined(typeof(StackItemType), type)) + return false; + if (instruction.OpCode != OpCode.NEWARRAY_T && type == StackItemType.Any) + return false; + break; + } + } + if (abi is null) return true; + foreach (ContractMethodDescriptor method in abi.Methods) + { + if (!instructions.ContainsKey(method.Offset)) + return false; + } + try + { + abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. + _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. + } + catch (ArgumentException) + { + return false; + } + return true; + } + public static UInt160 GetContractHash(UInt160 sender, uint nefCheckSum, string name) { using var sb = new ScriptBuilder(); diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 2130b9ee2a..51261c8fc5 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -4,7 +4,6 @@ using Neo.Network.P2P.Payloads; using Neo.Persistence; using Neo.SmartContract.Manifest; -using Neo.VM; using Neo.VM.Types; using System; using System.Collections.Generic; @@ -64,80 +63,6 @@ internal ContractManagement() Manifest.Abi.Events = events.ToArray(); } - private static void CheckScriptAndAbi(Script script, ContractAbi abi) - { - Dictionary instructions = new Dictionary(); - for (int ip = 0; ip < script.Length;) - { - Instruction instruction = script.GetInstruction(ip); - instructions.Add(ip, instruction); - ip += instruction.Size; - } - foreach (var (ip, instruction) in instructions) - { - switch (instruction.OpCode) - { - case OpCode.JMP: - case OpCode.JMPIF: - case OpCode.JMPIFNOT: - case OpCode.JMPEQ: - case OpCode.JMPNE: - case OpCode.JMPGT: - case OpCode.JMPGE: - case OpCode.JMPLT: - case OpCode.JMPLE: - case OpCode.CALL: - case OpCode.ENDTRY: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) - throw new ArgumentException(null, nameof(script)); - break; - case OpCode.PUSHA: - case OpCode.JMP_L: - case OpCode.JMPIF_L: - case OpCode.JMPIFNOT_L: - case OpCode.JMPEQ_L: - case OpCode.JMPNE_L: - case OpCode.JMPGT_L: - case OpCode.JMPGE_L: - case OpCode.JMPLT_L: - case OpCode.JMPLE_L: - case OpCode.CALL_L: - case OpCode.ENDTRY_L: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) - throw new ArgumentException(null, nameof(script)); - break; - case OpCode.TRY: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) - throw new ArgumentException(null, nameof(script)); - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8_1))) - throw new ArgumentException(null, nameof(script)); - break; - case OpCode.TRY_L: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) - throw new ArgumentException(null, nameof(script)); - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32_1))) - throw new ArgumentException(null, nameof(script)); - break; - case OpCode.NEWARRAY_T: - case OpCode.ISTYPE: - case OpCode.CONVERT: - StackItemType type = (StackItemType)instruction.TokenU8; - if (!Enum.IsDefined(typeof(StackItemType), type)) - throw new ArgumentException(null, nameof(script)); - if (instruction.OpCode != OpCode.NEWARRAY_T && type == StackItemType.Any) - throw new ArgumentException(null, nameof(script)); - break; - } - } - foreach (ContractMethodDescriptor method in abi.Methods) - { - if (!instructions.ContainsKey(method.Offset)) - throw new ArgumentException(null, nameof(script)); - } - abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. - _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. - } - private int GetNextAvailableId(DataCache snapshot) { StorageItem item = snapshot.GetAndChange(CreateStorageKey(Prefix_NextAvailableId)); @@ -218,7 +143,7 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma NefFile nef = nefFile.AsSerializable(); ContractManifest parsedManifest = ContractManifest.Parse(manifest); - CheckScriptAndAbi(nef.Script, parsedManifest.Abi); + Helper.Check(nef.Script, parsedManifest.Abi); UInt160 hash = Helper.GetContractHash(tx.Sender, nef.CheckSum, parsedManifest.Name); StorageKey key = CreateStorageKey(Prefix_Contract).Add(hash); if (engine.Snapshot.Contains(key)) @@ -282,7 +207,7 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}"); contract.Manifest = manifest_new; } - CheckScriptAndAbi(contract.Nef.Script, contract.Manifest.Abi); + Helper.Check(contract.Nef.Script, contract.Manifest.Abi); contract.UpdateCounter++; // Increase update counter if (nefFile != null) { From bfd06cc2d642474fadbf28451f702ceb37c002a8 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 23:54:07 +0800 Subject: [PATCH 05/15] Fix --- src/neo/SmartContract/Native/ContractManagement.cs | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 51261c8fc5..5f288791a3 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -143,7 +143,8 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma NefFile nef = nefFile.AsSerializable(); ContractManifest parsedManifest = ContractManifest.Parse(manifest); - Helper.Check(nef.Script, parsedManifest.Abi); + if (!Helper.Check(nef.Script, parsedManifest.Abi)) + throw new FormatException(); UInt160 hash = Helper.GetContractHash(tx.Sender, nef.CheckSum, parsedManifest.Name); StorageKey key = CreateStorageKey(Prefix_Contract).Add(hash); if (engine.Snapshot.Contains(key)) @@ -207,7 +208,8 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}"); contract.Manifest = manifest_new; } - Helper.Check(contract.Nef.Script, contract.Manifest.Abi); + if (!Helper.Check(contract.Nef.Script, contract.Manifest.Abi)) + throw new FormatException(); contract.UpdateCounter++; // Increase update counter if (nefFile != null) { From a13cbb59247ae2532990a53843d0b826120c4028 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Tue, 26 Jan 2021 23:58:00 +0800 Subject: [PATCH 06/15] Check all transactions and witnesses --- src/neo/Network/P2P/Payloads/Transaction.cs | 2 ++ src/neo/Network/P2P/Payloads/Witness.cs | 3 +++ 2 files changed, 5 insertions(+) diff --git a/src/neo/Network/P2P/Payloads/Transaction.cs b/src/neo/Network/P2P/Payloads/Transaction.cs index 2007c8c202..a859b36a76 100644 --- a/src/neo/Network/P2P/Payloads/Transaction.cs +++ b/src/neo/Network/P2P/Payloads/Transaction.cs @@ -12,6 +12,7 @@ using System.Collections.Generic; using System.IO; using System.Linq; +using static Neo.SmartContract.Helper; using Array = Neo.VM.Types.Array; namespace Neo.Network.P2P.Payloads @@ -199,6 +200,7 @@ public void DeserializeUnsigned(BinaryReader reader) Attributes = DeserializeAttributes(reader, MaxTransactionAttributes - Signers.Length).ToArray(); Script = reader.ReadVarBytes(ushort.MaxValue); if (Script.Length == 0) throw new FormatException(); + if (!Check(Script)) throw new FormatException(); } public bool Equals(Transaction other) diff --git a/src/neo/Network/P2P/Payloads/Witness.cs b/src/neo/Network/P2P/Payloads/Witness.cs index d6cfcf7470..2639f1a1bc 100644 --- a/src/neo/Network/P2P/Payloads/Witness.cs +++ b/src/neo/Network/P2P/Payloads/Witness.cs @@ -3,6 +3,7 @@ using Neo.SmartContract; using System; using System.IO; +using static Neo.SmartContract.Helper; namespace Neo.Network.P2P.Payloads { @@ -40,7 +41,9 @@ public virtual UInt160 ScriptHash void ISerializable.Deserialize(BinaryReader reader) { InvocationScript = reader.ReadVarBytes(MaxInvocationScript); + if (!Check(InvocationScript)) throw new FormatException(); VerificationScript = reader.ReadVarBytes(MaxVerificationScript); + if (!Check(VerificationScript)) throw new FormatException(); } void ISerializable.Serialize(BinaryWriter writer) From 35206a6ac210bc64b0a971819844cdeb02b24a08 Mon Sep 17 00:00:00 2001 From: Qiao Jin <43407364+Qiao-Jin@users.noreply.github.com> Date: Wed, 27 Jan 2021 17:05:47 +0800 Subject: [PATCH 07/15] fix 2266 (#2268) --- tests/neo.UnitTests/Ledger/UT_TransactionState.cs | 3 ++- tests/neo.UnitTests/Network/P2P/Payloads/UT_Block.cs | 10 +++++----- .../Network/P2P/Payloads/UT_ExtensiblePayload.cs | 3 ++- tests/neo.UnitTests/Network/P2P/UT_Message.cs | 10 ++++++---- .../SmartContract/UT_ContractParameterContext.cs | 6 +++--- tests/neo.UnitTests/TestUtils.cs | 2 +- 6 files changed, 19 insertions(+), 15 deletions(-) diff --git a/tests/neo.UnitTests/Ledger/UT_TransactionState.cs b/tests/neo.UnitTests/Ledger/UT_TransactionState.cs index 6d06e582a1..e57d3af1c4 100644 --- a/tests/neo.UnitTests/Ledger/UT_TransactionState.cs +++ b/tests/neo.UnitTests/Ledger/UT_TransactionState.cs @@ -3,6 +3,7 @@ using Neo.Network.P2P.Payloads; using Neo.SmartContract; using Neo.SmartContract.Native; +using Neo.VM; using System; using System.IO; @@ -22,7 +23,7 @@ public void Initialize() Transaction = new Transaction() { Attributes = Array.Empty(), - Script = new byte[] { 0x01 }, + Script = new byte[] { (byte)OpCode.PUSH1 }, Signers = new Signer[] { new Signer() { Account = UInt160.Zero } }, Witnesses = new Witness[] { new Witness() { InvocationScript=Array.Empty(), diff --git a/tests/neo.UnitTests/Network/P2P/Payloads/UT_Block.cs b/tests/neo.UnitTests/Network/P2P/Payloads/UT_Block.cs index 533f277f9a..a1ea5635c1 100644 --- a/tests/neo.UnitTests/Network/P2P/Payloads/UT_Block.cs +++ b/tests/neo.UnitTests/Network/P2P/Payloads/UT_Block.cs @@ -84,7 +84,7 @@ public void Serialize() UInt256 val256 = UInt256.Zero; TestUtils.SetupBlockWithValues(uut, val256, out var _, out var _, out var _, out var _, out var _, out var _, 1); - var hex = "000000000000000000000000000000000000000000000000000000000000000000000000add6632f6f3d29cdf94555bb191fb5296683e5446f9937c56bb94c8608023044e913ff854c00000000000000000000000000000000000000000000000000000001000111020000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000001000100010000"; + var hex = "0000000000000000000000000000000000000000000000000000000000000000000000007ee5991fa69cf4d7902430f5fad89ba13b253b5680cb13167f80bfc3593947e7e913ff854c00000000000000000000000000000000000000000000000000000001000111020000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000001000112010000"; uut.ToArray().ToHexString().Should().Be(hex); } @@ -94,7 +94,7 @@ public void Deserialize() UInt256 val256 = UInt256.Zero; TestUtils.SetupBlockWithValues(new Block(), val256, out var merkRoot, out var val160, out var timestampVal, out var indexVal, out var scriptVal, out var transactionsVal, 1); - var hex = "000000000000000000000000000000000000000000000000000000000000000000000000add6632f6f3d29cdf94555bb191fb5296683e5446f9937c56bb94c8608023044e913ff854c00000000000000000000000000000000000000000000000000000001000111020000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000001000100010000"; + var hex = "0000000000000000000000000000000000000000000000000000000000000000000000007ee5991fa69cf4d7902430f5fad89ba13b253b5680cb13167f80bfc3593947e7e913ff854c00000000000000000000000000000000000000000000000000000001000111020000000000000000000000000000000000000000000000000000000000000000000001000000000000000000000000000000000000000001000112010000"; using (MemoryStream ms = new MemoryStream(hex.HexToBytes(), false)) using (BinaryReader reader = new BinaryReader(ms)) @@ -178,11 +178,11 @@ public void ToJson() JObject jObj = uut.ToJson(); jObj.Should().NotBeNull(); - jObj["hash"].AsString().Should().Be("0x9a164d5b9a1ab8745c97dbaaaef8eb30b0d80a00205acdc82daf502bee69bc20"); + jObj["hash"].AsString().Should().Be("0xaf156a193ba2d7f05a37dd4e6fdfd1167cb6db3df5474df43a71a645a449fbc8"); jObj["size"].AsNumber().Should().Be(167); jObj["version"].AsNumber().Should().Be(0); jObj["previousblockhash"].AsString().Should().Be("0x0000000000000000000000000000000000000000000000000000000000000000"); - jObj["merkleroot"].AsString().Should().Be("0x44300208864cb96bc537996f44e5836629b51f19bb5545f9cd293d6f2f63d6ad"); + jObj["merkleroot"].AsString().Should().Be("0xe7473959c3bf807f1613cb80563b253ba19bd8faf5302490d7f49ca61f99e57e"); jObj["time"].AsNumber().Should().Be(328665601001); jObj["index"].AsNumber().Should().Be(0); jObj["nextconsensus"].AsString().Should().Be("NKuyBkoGdZZSLyPbJEetheRhMjeznFZszf"); @@ -193,7 +193,7 @@ public void ToJson() jObj["tx"].Should().NotBeNull(); JArray txObj = (JArray)jObj["tx"]; - txObj[0]["hash"].AsString().Should().Be("0x995ce8ff19c30f6b0d6b03e5ed8bd30b08027c92177923782d3a64f573421931"); + txObj[0]["hash"].AsString().Should().Be("0x0ee830a3b3e93679e23a9ee7e2a55287d79b6d67d056b03aa7ca917bd3d2923c"); txObj[0]["size"].AsNumber().Should().Be(53); txObj[0]["version"].AsNumber().Should().Be(0); ((JArray)txObj[0]["attributes"]).Count.Should().Be(0); diff --git a/tests/neo.UnitTests/Network/P2P/Payloads/UT_ExtensiblePayload.cs b/tests/neo.UnitTests/Network/P2P/Payloads/UT_ExtensiblePayload.cs index 0fd01642b1..77663bda41 100644 --- a/tests/neo.UnitTests/Network/P2P/Payloads/UT_ExtensiblePayload.cs +++ b/tests/neo.UnitTests/Network/P2P/Payloads/UT_ExtensiblePayload.cs @@ -3,6 +3,7 @@ using Neo.IO; using Neo.Network.P2P.Payloads; using Neo.SmartContract; +using Neo.VM; using System; namespace Neo.UnitTests.Network.P2P.Payloads @@ -33,7 +34,7 @@ public void DeserializeAndSerialize() ValidBlockEnd = 789, Sender = Array.Empty().ToScriptHash(), Data = new byte[] { 1, 2, 3 }, - Witness = new Witness() { InvocationScript = new byte[] { 3, 5, 6 }, VerificationScript = Array.Empty() } + Witness = new Witness() { InvocationScript = new byte[] { (byte)OpCode.PUSH1, (byte)OpCode.PUSH2, (byte)OpCode.PUSH3 }, VerificationScript = Array.Empty() } }; var clone = test.ToArray().AsSerializable(); diff --git a/tests/neo.UnitTests/Network/P2P/UT_Message.cs b/tests/neo.UnitTests/Network/P2P/UT_Message.cs index c70e0b37fd..e688806a75 100644 --- a/tests/neo.UnitTests/Network/P2P/UT_Message.cs +++ b/tests/neo.UnitTests/Network/P2P/UT_Message.cs @@ -4,6 +4,7 @@ using Neo.IO; using Neo.Network.P2P; using Neo.Network.P2P.Payloads; +using Neo.VM; using System; using System.Linq; @@ -131,7 +132,7 @@ public void Compression() Nonce = 1, Version = 0, Attributes = new TransactionAttribute[0], - Script = new byte[75], + Script = new byte[] { (byte)OpCode.PUSH1 }, Signers = new Signer[] { new Signer() { Account = UInt160.Zero } }, Witnesses = new Witness[0], }; @@ -139,13 +140,14 @@ public void Compression() var msg = Message.Create(MessageCommand.Transaction, payload); var buffer = msg.ToArray(); - buffer.Length.Should().Be(128); + buffer.Length.Should().Be(54); - payload.Script = new byte[payload.Script.Length + 10]; + payload.Script = new byte[100]; + for (int i = 0; i < payload.Script.Length; i++) payload.Script[i] = (byte)OpCode.PUSH2; msg = Message.Create(MessageCommand.Transaction, payload); buffer = msg.ToArray(); - buffer.Length.Should().Be(33); + buffer.Length.Should().Be(30); var copy = buffer.AsSerializable(); var payloadCopy = (Transaction)copy.Payload; diff --git a/tests/neo.UnitTests/SmartContract/UT_ContractParameterContext.cs b/tests/neo.UnitTests/SmartContract/UT_ContractParameterContext.cs index b96c06ae3c..fa6b4239db 100644 --- a/tests/neo.UnitTests/SmartContract/UT_ContractParameterContext.cs +++ b/tests/neo.UnitTests/SmartContract/UT_ContractParameterContext.cs @@ -45,15 +45,15 @@ public void TestToString() var context = new ContractParametersContext(tx); context.Add(contract, 0, new byte[] { 0x01 }); string str = context.ToString(); - str.Should().Be(@"{""type"":""Neo.Network.P2P.Payloads.Transaction"",""hex"":""AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFmUJDLobcPtqo9vZKIdjXsd8fVGwEAAQA="",""items"":{}}"); + str.Should().Be(@"{""type"":""Neo.Network.P2P.Payloads.Transaction"",""hex"":""AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFmUJDLobcPtqo9vZKIdjXsd8fVGwEAARI="",""items"":{}}"); } [TestMethod] public void TestParse() { - var ret = ContractParametersContext.Parse("{\"type\":\"Neo.Network.P2P.Payloads.Transaction\",\"hex\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFmUJDLobcPtqo9vZKIdjXsd8fVGwEAAQA=\",\"items\":{\"0xbecaad15c0ea585211faf99738a4354014f177f2\":{\"script\":\"IQJv8DuUkkHOHa3UNRnmlg4KhbQaaaBcMoEDqivOFZTKFmh0dHaq\",\"parameters\":[{\"type\":\"Signature\",\"value\":\"AQ==\"}]}}}"); + var ret = ContractParametersContext.Parse("{\"type\":\"Neo.Network.P2P.Payloads.Transaction\",\"hex\":\"AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAFmUJDLobcPtqo9vZKIdjXsd8fVGwEAARI=\",\"items\":{\"0xbecaad15c0ea585211faf99738a4354014f177f2\":{\"script\":\"IQJv8DuUkkHOHa3UNRnmlg4KhbQaaaBcMoEDqivOFZTKFmh0dHaq\",\"parameters\":[{\"type\":\"Signature\",\"value\":\"AQ==\"}]}}}"); ret.ScriptHashes[0].ToString().Should().Be("0x1bd5c777ec35768892bd3daab60fb7a1cb905066"); - ((Transaction)ret.Verifiable).Script.ToHexString().Should().Be(new byte[1].ToHexString()); + ((Transaction)ret.Verifiable).Script.ToHexString().Should().Be(new byte[] { 18 }.ToHexString()); } [TestMethod] diff --git a/tests/neo.UnitTests/TestUtils.cs b/tests/neo.UnitTests/TestUtils.cs index f68e058d01..d68ba3a93f 100644 --- a/tests/neo.UnitTests/TestUtils.cs +++ b/tests/neo.UnitTests/TestUtils.cs @@ -103,7 +103,7 @@ public static Transaction GetTransaction(UInt160 sender) { return new Transaction { - Script = new byte[1], + Script = new byte[] { (byte)OpCode.PUSH2 }, Attributes = Array.Empty(), Signers = new[]{ new Signer() { From f46d42e2aacaaf16c3b34a50c234d7f693e8402c Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Wed, 27 Jan 2021 17:11:23 +0800 Subject: [PATCH 08/15] Move the check of Transaction.Script to VerifyStateIndependent --- src/neo/Network/P2P/Payloads/Transaction.cs | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/src/neo/Network/P2P/Payloads/Transaction.cs b/src/neo/Network/P2P/Payloads/Transaction.cs index a859b36a76..58c2d8f456 100644 --- a/src/neo/Network/P2P/Payloads/Transaction.cs +++ b/src/neo/Network/P2P/Payloads/Transaction.cs @@ -200,7 +200,6 @@ public void DeserializeUnsigned(BinaryReader reader) Attributes = DeserializeAttributes(reader, MaxTransactionAttributes - Signers.Length).ToArray(); Script = reader.ReadVarBytes(ushort.MaxValue); if (Script.Length == 0) throw new FormatException(); - if (!Check(Script)) throw new FormatException(); } public bool Equals(Transaction other) @@ -322,13 +321,13 @@ public virtual VerifyResult VerifyStateDependent(DataCache snapshot, Transaction public virtual VerifyResult VerifyStateIndependent() { - if (Size > MaxTransactionSize) - return VerifyResult.Invalid; + if (Size > MaxTransactionSize) return VerifyResult.Invalid; + if (!Check(Script)) return VerifyResult.Invalid; UInt160[] hashes = GetScriptHashesForVerifying(null); if (hashes.Length != witnesses.Length) return VerifyResult.Invalid; for (int i = 0; i < hashes.Length; i++) if (witnesses[i].VerificationScript.IsStandardContract()) - if (!this.VerifyWitness(null, hashes[i], witnesses[i], SmartContract.Helper.MaxVerificationGas, out _)) + if (!this.VerifyWitness(null, hashes[i], witnesses[i], MaxVerificationGas, out _)) return VerifyResult.Invalid; return VerifyResult.Succeed; } From 4649a661eaf6e889d9bd6310f62e7e44d4b57ba5 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Wed, 27 Jan 2021 17:15:19 +0800 Subject: [PATCH 09/15] Avoid exceptions --- src/neo/SmartContract/Helper.cs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 71dbb2baf3..251593b8b0 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -35,7 +35,15 @@ public static bool Check(Script script, ContractAbi abi = null) Dictionary instructions = new Dictionary(); for (int ip = 0; ip < script.Length;) { - Instruction instruction = script.GetInstruction(ip); + Instruction instruction; + try + { + instruction = script.GetInstruction(ip); + } + catch (InvalidOperationException) + { + return false; + } instructions.Add(ip, instruction); ip += instruction.Size; } From 17f3556fd08a763891efee341a119574e1ae6f6f Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Wed, 27 Jan 2021 17:21:58 +0800 Subject: [PATCH 10/15] Move check of Witness to VerifyWitness --- src/neo/Network/P2P/Payloads/Witness.cs | 3 --- src/neo/SmartContract/Helper.cs | 2 ++ 2 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/neo/Network/P2P/Payloads/Witness.cs b/src/neo/Network/P2P/Payloads/Witness.cs index 2639f1a1bc..d6cfcf7470 100644 --- a/src/neo/Network/P2P/Payloads/Witness.cs +++ b/src/neo/Network/P2P/Payloads/Witness.cs @@ -3,7 +3,6 @@ using Neo.SmartContract; using System; using System.IO; -using static Neo.SmartContract.Helper; namespace Neo.Network.P2P.Payloads { @@ -41,9 +40,7 @@ public virtual UInt160 ScriptHash void ISerializable.Deserialize(BinaryReader reader) { InvocationScript = reader.ReadVarBytes(MaxInvocationScript); - if (!Check(InvocationScript)) throw new FormatException(); VerificationScript = reader.ReadVarBytes(MaxVerificationScript); - if (!Check(VerificationScript)) throw new FormatException(); } void ISerializable.Serialize(BinaryWriter writer) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 251593b8b0..1d2591928b 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -280,6 +280,8 @@ public static bool VerifyWitnesses(this IVerifiable verifiable, DataCache snapsh internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapshot, UInt160 hash, Witness witness, long gas, out long fee) { fee = 0; + if (!Check(witness.InvocationScript)) return false; + if (!Check(witness.VerificationScript)) return false; using (ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, verifiable, snapshot?.CreateSnapshot(), null, gas)) { CallFlags callFlags = !witness.VerificationScript.IsStandardContract() ? CallFlags.ReadStates : CallFlags.None; From bebef04a1f1a831c6005b77375ab783f66b4c6b6 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Wed, 27 Jan 2021 17:24:36 +0800 Subject: [PATCH 11/15] Fix UT --- tests/neo.UnitTests/SmartContract/UT_SmartContractHelper.cs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/neo.UnitTests/SmartContract/UT_SmartContractHelper.cs b/tests/neo.UnitTests/SmartContract/UT_SmartContractHelper.cs index 970f29720a..c3d3209588 100644 --- a/tests/neo.UnitTests/SmartContract/UT_SmartContractHelper.cs +++ b/tests/neo.UnitTests/SmartContract/UT_SmartContractHelper.cs @@ -151,7 +151,7 @@ public void TestVerifyWitnesses() Witness = new Witness() { InvocationScript = new byte[0], VerificationScript = new byte[0] } }; BlocksAdd(snapshot2, index2, block2); - Header header2 = new Header() { PrevHash = index2, Witness = new Witness { VerificationScript = new byte[0] } }; + Header header2 = new Header() { PrevHash = index2, Witness = new Witness { InvocationScript = new byte[0], VerificationScript = new byte[0] } }; snapshot2.AddContract(UInt160.Zero, new ContractState()); snapshot2.DeleteContract(UInt160.Zero); From 96fd06f410e02c826d0200007a5a5ca2d41763e9 Mon Sep 17 00:00:00 2001 From: Shargon Date: Wed, 27 Jan 2021 19:42:57 +0100 Subject: [PATCH 12/15] Optimization: Reuse Script class --- src/neo/SmartContract/Helper.cs | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 1d2591928b..9baf41b2c3 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -280,14 +280,15 @@ public static bool VerifyWitnesses(this IVerifiable verifiable, DataCache snapsh internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapshot, UInt160 hash, Witness witness, long gas, out long fee) { fee = 0; - if (!Check(witness.InvocationScript)) return false; - if (!Check(witness.VerificationScript)) return false; + Script invocationScript = witness.InvocationScript; + if (!Check(invocationScript)) return false; + Script verificationScript = witness.VerificationScript; + if (!Check(verificationScript)) return false; using (ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, verifiable, snapshot?.CreateSnapshot(), null, gas)) { CallFlags callFlags = !witness.VerificationScript.IsStandardContract() ? CallFlags.ReadStates : CallFlags.None; - byte[] verification = witness.VerificationScript; - if (verification.Length == 0) + if (verificationScript.Length == 0) { ContractState cs = NativeContract.ContractManagement.GetContract(snapshot, hash); if (cs is null) return false; @@ -299,14 +300,14 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh { if (NativeContract.IsNative(hash)) return false; if (hash != witness.ScriptHash) return false; - engine.LoadScript(verification, initialPosition: 0, configureState: p => + engine.LoadScript(verificationScript, initialPosition: 0, configureState: p => { p.CallFlags = callFlags; p.ScriptHash = hash; }); } - engine.LoadScript(witness.InvocationScript, configureState: p => p.CallFlags = CallFlags.None); + engine.LoadScript(invocationScript, configureState: p => p.CallFlags = CallFlags.None); if (NativeContract.IsNative(hash)) { From c3768aecab19ff75ce24ecdbafc344802c2c2197 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 28 Jan 2021 18:22:26 +0800 Subject: [PATCH 13/15] Neo.VM 3.0.0-CI00262 --- src/neo/Network/P2P/Payloads/Transaction.cs | 9 +- src/neo/SmartContract/Helper.cs | 114 ++++-------------- .../Native/ContractManagement.cs | 6 +- src/neo/neo.csproj | 2 +- 4 files changed, 36 insertions(+), 95 deletions(-) diff --git a/src/neo/Network/P2P/Payloads/Transaction.cs b/src/neo/Network/P2P/Payloads/Transaction.cs index 58c2d8f456..b628fe1451 100644 --- a/src/neo/Network/P2P/Payloads/Transaction.cs +++ b/src/neo/Network/P2P/Payloads/Transaction.cs @@ -322,7 +322,14 @@ public virtual VerifyResult VerifyStateDependent(DataCache snapshot, Transaction public virtual VerifyResult VerifyStateIndependent() { if (Size > MaxTransactionSize) return VerifyResult.Invalid; - if (!Check(Script)) return VerifyResult.Invalid; + try + { + Check(Script); + } + catch (BadScriptException) + { + return VerifyResult.Invalid; + } UInt160[] hashes = GetScriptHashesForVerifying(null); if (hashes.Length != witnesses.Length) return VerifyResult.Invalid; for (int i = 0; i < hashes.Length; i++) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 9baf41b2c3..38a375e656 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -30,95 +30,17 @@ public static long MultiSignatureContractCost(int m, int n) => ApplicationEngine.OpCodePrices[OpCode.SYSCALL] + ApplicationEngine.ECDsaVerifyPrice * n; - public static bool Check(Script script, ContractAbi abi = null) + public static Script Check(byte[] script, ContractAbi abi = null) { - Dictionary instructions = new Dictionary(); - for (int ip = 0; ip < script.Length;) - { - Instruction instruction; - try - { - instruction = script.GetInstruction(ip); - } - catch (InvalidOperationException) - { - return false; - } - instructions.Add(ip, instruction); - ip += instruction.Size; - } - foreach (var (ip, instruction) in instructions) - { - switch (instruction.OpCode) - { - case OpCode.JMP: - case OpCode.JMPIF: - case OpCode.JMPIFNOT: - case OpCode.JMPEQ: - case OpCode.JMPNE: - case OpCode.JMPGT: - case OpCode.JMPGE: - case OpCode.JMPLT: - case OpCode.JMPLE: - case OpCode.CALL: - case OpCode.ENDTRY: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) - return false; - break; - case OpCode.PUSHA: - case OpCode.JMP_L: - case OpCode.JMPIF_L: - case OpCode.JMPIFNOT_L: - case OpCode.JMPEQ_L: - case OpCode.JMPNE_L: - case OpCode.JMPGT_L: - case OpCode.JMPGE_L: - case OpCode.JMPLT_L: - case OpCode.JMPLE_L: - case OpCode.CALL_L: - case OpCode.ENDTRY_L: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) - return false; - break; - case OpCode.TRY: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8))) - return false; - if (!instructions.ContainsKey(checked(ip + instruction.TokenI8_1))) - return false; - break; - case OpCode.TRY_L: - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32))) - return false; - if (!instructions.ContainsKey(checked(ip + instruction.TokenI32_1))) - return false; - break; - case OpCode.NEWARRAY_T: - case OpCode.ISTYPE: - case OpCode.CONVERT: - StackItemType type = (StackItemType)instruction.TokenU8; - if (!Enum.IsDefined(typeof(StackItemType), type)) - return false; - if (instruction.OpCode != OpCode.NEWARRAY_T && type == StackItemType.Any) - return false; - break; - } - } - if (abi is null) return true; - foreach (ContractMethodDescriptor method in abi.Methods) - { - if (!instructions.ContainsKey(method.Offset)) - return false; - } - try + Script s = new Script(script, true); + if (abi is not null) { + foreach (ContractMethodDescriptor method in abi.Methods) + s.GetInstruction(method.Offset); abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. } - catch (ArgumentException) - { - return false; - } - return true; + return s; } public static UInt160 GetContractHash(UInt160 sender, uint nefCheckSum, string name) @@ -280,15 +202,20 @@ public static bool VerifyWitnesses(this IVerifiable verifiable, DataCache snapsh internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapshot, UInt160 hash, Witness witness, long gas, out long fee) { fee = 0; - Script invocationScript = witness.InvocationScript; - if (!Check(invocationScript)) return false; - Script verificationScript = witness.VerificationScript; - if (!Check(verificationScript)) return false; + Script invocationScript; + try + { + invocationScript = Check(witness.InvocationScript); + } + catch (BadScriptException) + { + return false; + } using (ApplicationEngine engine = ApplicationEngine.Create(TriggerType.Verification, verifiable, snapshot?.CreateSnapshot(), null, gas)) { CallFlags callFlags = !witness.VerificationScript.IsStandardContract() ? CallFlags.ReadStates : CallFlags.None; - if (verificationScript.Length == 0) + if (witness.VerificationScript.Length == 0) { ContractState cs = NativeContract.ContractManagement.GetContract(snapshot, hash); if (cs is null) return false; @@ -300,6 +227,15 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh { if (NativeContract.IsNative(hash)) return false; if (hash != witness.ScriptHash) return false; + Script verificationScript; + try + { + verificationScript = Check(witness.VerificationScript); + } + catch (BadScriptException) + { + return false; + } engine.LoadScript(verificationScript, initialPosition: 0, configureState: p => { p.CallFlags = callFlags; diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 5f288791a3..51261c8fc5 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -143,8 +143,7 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma NefFile nef = nefFile.AsSerializable(); ContractManifest parsedManifest = ContractManifest.Parse(manifest); - if (!Helper.Check(nef.Script, parsedManifest.Abi)) - throw new FormatException(); + Helper.Check(nef.Script, parsedManifest.Abi); UInt160 hash = Helper.GetContractHash(tx.Sender, nef.CheckSum, parsedManifest.Name); StorageKey key = CreateStorageKey(Prefix_Contract).Add(hash); if (engine.Snapshot.Contains(key)) @@ -208,8 +207,7 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}"); contract.Manifest = manifest_new; } - if (!Helper.Check(contract.Nef.Script, contract.Manifest.Abi)) - throw new FormatException(); + Helper.Check(contract.Nef.Script, contract.Manifest.Abi); contract.UpdateCounter++; // Increase update counter if (nefFile != null) { diff --git a/src/neo/neo.csproj b/src/neo/neo.csproj index f079bc9509..224a8fde2b 100644 --- a/src/neo/neo.csproj +++ b/src/neo/neo.csproj @@ -29,7 +29,7 @@ - + From 5fab37297f56a14ce5ee89829e5f7ba467b67185 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 28 Jan 2021 18:29:52 +0800 Subject: [PATCH 14/15] Optimize --- src/neo/Network/P2P/Payloads/Transaction.cs | 5 ++--- src/neo/SmartContract/Helper.cs | 18 ++---------------- .../SmartContract/Native/ContractManagement.cs | 14 ++++++++++++-- 3 files changed, 16 insertions(+), 21 deletions(-) diff --git a/src/neo/Network/P2P/Payloads/Transaction.cs b/src/neo/Network/P2P/Payloads/Transaction.cs index b628fe1451..5d8ffa4b3a 100644 --- a/src/neo/Network/P2P/Payloads/Transaction.cs +++ b/src/neo/Network/P2P/Payloads/Transaction.cs @@ -12,7 +12,6 @@ using System.Collections.Generic; using System.IO; using System.Linq; -using static Neo.SmartContract.Helper; using Array = Neo.VM.Types.Array; namespace Neo.Network.P2P.Payloads @@ -324,7 +323,7 @@ public virtual VerifyResult VerifyStateIndependent() if (Size > MaxTransactionSize) return VerifyResult.Invalid; try { - Check(Script); + _ = new Script(Script, true); } catch (BadScriptException) { @@ -334,7 +333,7 @@ public virtual VerifyResult VerifyStateIndependent() if (hashes.Length != witnesses.Length) return VerifyResult.Invalid; for (int i = 0; i < hashes.Length; i++) if (witnesses[i].VerificationScript.IsStandardContract()) - if (!this.VerifyWitness(null, hashes[i], witnesses[i], MaxVerificationGas, out _)) + if (!this.VerifyWitness(null, hashes[i], witnesses[i], SmartContract.Helper.MaxVerificationGas, out _)) return VerifyResult.Invalid; return VerifyResult.Succeed; } diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 38a375e656..174570beda 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -9,7 +9,6 @@ using System; using System.Buffers.Binary; using System.Collections.Generic; -using System.Linq; namespace Neo.SmartContract { @@ -30,19 +29,6 @@ public static long MultiSignatureContractCost(int m, int n) => ApplicationEngine.OpCodePrices[OpCode.SYSCALL] + ApplicationEngine.ECDsaVerifyPrice * n; - public static Script Check(byte[] script, ContractAbi abi = null) - { - Script s = new Script(script, true); - if (abi is not null) - { - foreach (ContractMethodDescriptor method in abi.Methods) - s.GetInstruction(method.Offset); - abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. - _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. - } - return s; - } - public static UInt160 GetContractHash(UInt160 sender, uint nefCheckSum, string name) { using var sb = new ScriptBuilder(); @@ -205,7 +191,7 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh Script invocationScript; try { - invocationScript = Check(witness.InvocationScript); + invocationScript = new Script(witness.InvocationScript); } catch (BadScriptException) { @@ -230,7 +216,7 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh Script verificationScript; try { - verificationScript = Check(witness.VerificationScript); + verificationScript = new Script(witness.VerificationScript); } catch (BadScriptException) { diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 51261c8fc5..9d61edef62 100644 --- a/src/neo/SmartContract/Native/ContractManagement.cs +++ b/src/neo/SmartContract/Native/ContractManagement.cs @@ -4,6 +4,7 @@ using Neo.Network.P2P.Payloads; using Neo.Persistence; using Neo.SmartContract.Manifest; +using Neo.VM; using Neo.VM.Types; using System; using System.Collections.Generic; @@ -63,6 +64,15 @@ internal ContractManagement() Manifest.Abi.Events = events.ToArray(); } + private static void Check(byte[] script, ContractAbi abi) + { + Script s = new Script(script, true); + foreach (ContractMethodDescriptor method in abi.Methods) + s.GetInstruction(method.Offset); + abi.GetMethod(string.Empty, 0); // Trigger the construction of ContractAbi.methodDictionary to check the uniqueness of the method names. + _ = abi.Events.ToDictionary(p => p.Name); // Check the uniqueness of the event names. + } + private int GetNextAvailableId(DataCache snapshot) { StorageItem item = snapshot.GetAndChange(CreateStorageKey(Prefix_NextAvailableId)); @@ -143,7 +153,7 @@ private ContractState Deploy(ApplicationEngine engine, byte[] nefFile, byte[] ma NefFile nef = nefFile.AsSerializable(); ContractManifest parsedManifest = ContractManifest.Parse(manifest); - Helper.Check(nef.Script, parsedManifest.Abi); + Check(nef.Script, parsedManifest.Abi); UInt160 hash = Helper.GetContractHash(tx.Sender, nef.CheckSum, parsedManifest.Name); StorageKey key = CreateStorageKey(Prefix_Contract).Add(hash); if (engine.Snapshot.Contains(key)) @@ -207,7 +217,7 @@ private void Update(ApplicationEngine engine, byte[] nefFile, byte[] manifest, S throw new InvalidOperationException($"Invalid Manifest Hash: {contract.Hash}"); contract.Manifest = manifest_new; } - Helper.Check(contract.Nef.Script, contract.Manifest.Abi); + Check(contract.Nef.Script, contract.Manifest.Abi); contract.UpdateCounter++; // Increase update counter if (nefFile != null) { From 5c5f25651504bbb80c783e81f8b4110e81665f29 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Thu, 28 Jan 2021 21:16:48 +0800 Subject: [PATCH 15/15] Apply suggestions from code review Co-authored-by: Shargon --- src/neo/SmartContract/Helper.cs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/neo/SmartContract/Helper.cs b/src/neo/SmartContract/Helper.cs index 174570beda..7d80688934 100644 --- a/src/neo/SmartContract/Helper.cs +++ b/src/neo/SmartContract/Helper.cs @@ -191,7 +191,7 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh Script invocationScript; try { - invocationScript = new Script(witness.InvocationScript); + invocationScript = new Script(witness.InvocationScript, true); } catch (BadScriptException) { @@ -216,7 +216,7 @@ internal static bool VerifyWitness(this IVerifiable verifiable, DataCache snapsh Script verificationScript; try { - verificationScript = new Script(witness.VerificationScript); + verificationScript = new Script(witness.VerificationScript, true); } catch (BadScriptException) {