Merge branch 'master-n3' into n3-fix-4316

Author: ajara87

src/Neo/SmartContract/ApplicationEngine.cs

@@ -288,6 +288,10 @@ public static JumpTable ComposeNotGorgonJumpTable()
             table[OpCode.SETITEM] = SetItem_Before543;
             table[OpCode.REMOVE] = Remove_Before543;
 
+            // Before https://github.com/neo-project/neo-vm/pull/567.
+            table[OpCode.SHR] = VulnerableSHR;
+            table[OpCode.SHL] = VulnerableSHL;
+
             return table;
         }
 
@@ -711,6 +715,42 @@ private static void VulnerableSubStr(ExecutionEngine engine, Instruction instruc
             engine.Push(result);
         }
 
+        /// <summary>
+        /// Computes the left shift of an integer. Vulnerable implementation of
+        /// <see cref="OpCode.SHL"/> since it doesn't pop operand from stack in
+        /// case of zero shift.
+        /// </summary>
+        /// <param name="engine">The execution engine.</param>
+        /// <param name="instruction">The instruction being executed.</param>
+        /// <remarks>Pop 2, Push 1</remarks>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static void VulnerableSHL(ExecutionEngine engine, Instruction instruction)
+        {
+            var shift = (int)engine.Pop().GetInteger();
+            engine.Limits.AssertShift(shift);
+            if (shift == 0) return;
+            var x = engine.Pop().GetInteger();
+            engine.Push(x << shift);
+        }
+
+        /// <summary>
+        /// Computes the right shift of an integer. Vulnerable implementation of
+        /// <see cref="OpCode.SHR"/> since it doesn't pop operand from stack in
+        /// case of zero shift.
+        /// </summary>
+        /// <param name="engine">The execution engine.</param>
+        /// <param name="instruction">The instruction being executed.</param>
+        /// <remarks>Pop 2, Push 1</remarks>
+        [MethodImpl(MethodImplOptions.AggressiveInlining)]
+        private static void VulnerableSHR(ExecutionEngine engine, Instruction instruction)
+        {
+            var shift = (int)engine.Pop().GetInteger();
+            engine.Limits.AssertShift(shift);
+            if (shift == 0) return;
+            var x = engine.Pop().GetInteger();
+            engine.Push(x >> shift);
+        }
+
         public override void LoadContext(ExecutionContext context)
         {
             // Set default execution context state

tests/Neo.UnitTests/SmartContract/UT_ApplicationEngine.JumpTable.cs

@@ -20,6 +20,54 @@ namespace Neo.UnitTests.SmartContract
 {
     public partial class UT_ApplicationEngine
     {
+        [TestMethod]
+        public void TestSHR_SHL_WithDifferentHF()
+        {
+            foreach (var op in new OpCode[] { OpCode.SHL, OpCode.SHR })
+            {
+                using var sb = new ScriptBuilder();
+
+                sb.Emit(OpCode.NEWARRAY0); // intentionally wrong argument for SHL or SHR.
+                sb.EmitPush(0); // zero shift.
+                sb.Emit(op);
+
+                var script = sb.ToArray();
+
+                const uint EchidnaEnable = 10u;
+                const uint GorgonEnable = 20u;
+                // Hardfork heights:
+                // Echidna at 10, Gorgon at 20
+                //  - index=5 => pre-Echidna (NotEchidnaJumpTable)
+                //  - index=15 => Echidna enabled, Gorgon NOT enabled (NotGorgonJumpTable)
+                //  - index=30 => Gorgon enabled (DefaultJumpTable)
+                var settings = ProtocolSettings.Default with
+                {
+                    Hardforks = ProtocolSettings.Default.Hardforks
+                        .SetItem(Hardfork.HF_Echidna, EchidnaEnable)
+                        .SetItem(Hardfork.HF_Gorgon, GorgonEnable)
+                };
+
+                Assert.IsFalse(settings.IsHardforkEnabled(Hardfork.HF_Echidna, 5u));
+                Assert.IsTrue(settings.IsHardforkEnabled(Hardfork.HF_Echidna, 15u));
+                Assert.IsTrue(settings.IsHardforkEnabled(Hardfork.HF_Echidna, 30u));
+                Assert.IsFalse(settings.IsHardforkEnabled(Hardfork.HF_Gorgon, 15u));
+                Assert.IsTrue(settings.IsHardforkEnabled(Hardfork.HF_Gorgon, 30u));
+
+                // Case A: pre-Echidna => no exception.
+                var engine = Execute(script, settings, index: 5u);
+                Assert.AreEqual(VMState.HALT, engine.State);
+                Assert.AreEqual(1, engine.ResultStack.Count);
+
+                // Case B: Echidna enabled but pre-Gorgon => no exception.
+                engine = Execute(script, settings, index: 15u);
+                Assert.AreEqual(VMState.HALT, engine.State);
+                Assert.AreEqual(1, engine.ResultStack.Count);
+
+                // Case C: Gorgon enabled => InvalidCastException on attempt to convert Array to Integer.
+                ExecuteAndAssertFault<InvalidCastException>(script, settings, index: 30u);
+            }
+        }
+
         [TestMethod]
         public void TestHasKeyWithDifferentHF()
         {
@@ -76,6 +124,16 @@ private static byte[] BuildHasKeyLargeIndexScript()
         }
 
         private static void ExecuteAndAssertFault<TException>(byte[] script, ProtocolSettings settings, uint index) where TException : Exception
+        {
+            var engine = Execute(script, settings, index);
+
+            Assert.AreEqual(VMState.FAULT, engine.State, $"Expected FAULT at index={index}.");
+            Assert.IsNotNull(engine.FaultException, $"Expected FaultException at index={index}.");
+            Assert.IsInstanceOfType(engine.FaultException, typeof(TException),
+                $"Expected {typeof(TException).Name} at index={index}, but got {engine.FaultException.GetType().Name}.");
+        }
+
+        private static ApplicationEngine Execute(byte[] script, ProtocolSettings settings, uint index)
         {
             var snapshotCache = TestBlockchain.GetTestSnapshotCache();
             var block = new Block
@@ -102,10 +160,7 @@ private static void ExecuteAndAssertFault<TException>(byte[] script, ProtocolSet
             engine.LoadScript(script);
             engine.Execute();
 
-            Assert.AreEqual(VMState.FAULT, engine.State, $"Expected FAULT at index={index}.");
-            Assert.IsNotNull(engine.FaultException, $"Expected FaultException at index={index}.");
-            Assert.IsInstanceOfType(engine.FaultException, typeof(TException),
-                $"Expected {typeof(TException).Name} at index={index}, but got {engine.FaultException.GetType().Name}.");
+            return engine;
         }
     }
 }