From 97707480b656ad1e2787ffaf1a373397c0643e84 Mon Sep 17 00:00:00 2001 From: Erik Zhang Date: Fri, 29 Jan 2021 01:23:13 +0800 Subject: [PATCH] Check all transactions and witnesses (#2266) --- src/neo/Network/P2P/Payloads/Transaction.cs | 9 +- src/neo/SmartContract/Helper.cs | 109 ++++-------------- .../Native/ContractManagement.cs | 16 ++- src/neo/neo.csproj | 2 +- .../Ledger/UT_TransactionState.cs | 3 +- .../Network/P2P/Payloads/UT_Block.cs | 10 +- .../P2P/Payloads/UT_ExtensiblePayload.cs | 3 +- tests/neo.UnitTests/Network/P2P/UT_Message.cs | 10 +- .../UT_ContractParameterContext.cs | 6 +- .../SmartContract/UT_SmartContractHelper.cs | 2 +- tests/neo.UnitTests/TestUtils.cs | 2 +- 11 files changed, 62 insertions(+), 110 deletions(-) diff --git a/src/neo/Network/P2P/Payloads/Transaction.cs b/src/neo/Network/P2P/Payloads/Transaction.cs index 2007c8c202..5d8ffa4b3a 100644 --- a/src/neo/Network/P2P/Payloads/Transaction.cs +++ b/src/neo/Network/P2P/Payloads/Transaction.cs @@ -320,8 +320,15 @@ public virtual VerifyResult VerifyStateDependent(DataCache snapshot, Transaction public virtual VerifyResult VerifyStateIndependent() { - if (Size > MaxTransactionSize) + if (Size > MaxTransactionSize) return VerifyResult.Invalid; + try + { + _ = new Script(Script, true); + } + 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 71dbb2baf3..7d80688934 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,89 +29,6 @@ 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(); @@ -272,12 +188,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; + try + { + invocationScript = new Script(witness.InvocationScript, true); + } + 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; - byte[] verification = witness.VerificationScript; - if (verification.Length == 0) + if (witness.VerificationScript.Length == 0) { ContractState cs = NativeContract.ContractManagement.GetContract(snapshot, hash); if (cs is null) return false; @@ -289,14 +213,23 @@ 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 => + Script verificationScript; + try + { + verificationScript = new Script(witness.VerificationScript, true); + } + catch (BadScriptException) + { + return false; + } + 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)) { diff --git a/src/neo/SmartContract/Native/ContractManagement.cs b/src/neo/SmartContract/Native/ContractManagement.cs index 5f288791a3..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,8 +153,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(); + 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 +217,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(); + 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 @@ - + 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/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); 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() {