core/evm, contracts: avoid copying memory for input in calls + make e…

…crecover not modify input buffer
ethereum · Oct 18, 2019 · 8d39aed · 8d39aed
1 parent d5b79e7
commit 8d39aed
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 6 deletions.
diff --git a/core/vm/contracts.go b/core/vm/contracts.go
@@ -106,8 +106,13 @@ func (c *ecrecover) Run(input []byte) ([]byte, error) {
 	if !allZero(input[32:63]) || !crypto.ValidateSignatureValues(v, r, s, false) {
 		return nil, nil
 	}
+	// We must make sure not to modify the 'input', so placing the 'v' along with
+	// the signature needs to be done on a new allocation
+	sig := make([]byte, 65)
+	copy(sig, input[64:128])
+	sig[64] = v
 	// v needs to be at the end for libsecp256k1
-	pubKey, err := crypto.Ecrecover(input[:32], append(input[64:128], v))
+	pubKey, err := crypto.Ecrecover(input[:32], sig)
 	// make sure the public key is a valid one
 	if err != nil {
 		return nil, nil

diff --git a/core/vm/contracts_test.go b/core/vm/contracts_test.go
@@ -17,6 +17,7 @@
 package vm
 
 import (
+	"bytes"
 	"fmt"
 	"math/big"
 	"reflect"
@@ -409,6 +410,11 @@ func testPrecompiled(addr string, test precompiledTest, t *testing.T) {
 		} else if common.Bytes2Hex(res) != test.expected {
 			t.Errorf("Expected %v, got %v", test.expected, common.Bytes2Hex(res))
 		}
+		// Verify that the precompile did not touch the input buffer
+		exp := common.Hex2Bytes(test.input)
+		if !bytes.Equal(in, exp) {
+			t.Errorf("Precompiled %v modified input data", addr)
+		}
 	})
 }
 
@@ -423,6 +429,11 @@ func testPrecompiledFailure(addr string, test precompiledFailureTest, t *testing
 		if !reflect.DeepEqual(err, test.expectedError) {
 			t.Errorf("Expected error [%v], got [%v]", test.expectedError, err)
 		}
+		// Verify that the precompile did not touch the input buffer
+		exp := common.Hex2Bytes(test.input)
+		if !bytes.Equal(in, exp) {
+			t.Errorf("Precompiled %v modified input data", addr)
+		}
 	})
 }
 
@@ -574,3 +585,55 @@ func TestPrecompileBlake2FMalformedInput(t *testing.T) {
 		testPrecompiledFailure("09", test, t)
 	}
 }
+
+// EcRecover test vectors
+var ecRecoverTests = []precompiledTest{
+	{
+		input: "a8b53bdf3306a35a7103ab5504a0c9b492295564b6202b1942a84ef300107281" +
+			"000000000000000000000000000000000000000000000000000000000000001b" +
+			"3078356531653033663533636531386237373263636230303933666637316633" +
+			"6635336635633735623734646362333161383561613862383839326234653862" +
+			"1122334455667788991011121314151617181920212223242526272829303132",
+		expected: "",
+		name:     "CallEcrecoverUnrecoverableKey",
+	},
+	{
+		input: "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c" +
+			"000000000000000000000000000000000000000000000000000000000000001c" +
+			"73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f" +
+			"eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
+		expected: "000000000000000000000000a94f5374fce5edbc8e2a8697c15331677e6ebf0b",
+		name:     "ValidKey",
+	},
+	{
+		input: "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c" +
+			"100000000000000000000000000000000000000000000000000000000000001c" +
+			"73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f" +
+			"eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
+		expected: "",
+		name:     "InvalidHighV-bits-1",
+	},
+	{
+		input: "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c" +
+			"000000000000000000000000000000000000001000000000000000000000001c" +
+			"73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f" +
+			"eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
+		expected: "",
+		name:     "InvalidHighV-bits-2",
+	},
+	{
+		input: "18c547e4f7b0f325ad1e56f57e26c745b09a3e503d86e00e5255ff7f715d3d1c" +
+			"000000000000000000000000000000000000001000000000000000000000011c" +
+			"73b1693892219d736caba55bdb67216e485557ea6b6af75f37096c9aa6a5a75f" +
+			"eeb940b1d03b21e36b0e47e79769f095fe2ab855bd91e3a38756b7d75a9c4549",
+		expected: "",
+		name:     "InvalidHighV-bits-3",
+	},
+}
+
+func TestPrecompiledEcrecover(t *testing.T) {
+	for _, test := range ecRecoverTests {
+		testPrecompiled("01", test, t)
+	}
+
+}
diff --git a/core/vm/instructions.go b/core/vm/instructions.go
@@ -384,7 +384,7 @@ func opSAR(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *
 
 func opSha3(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
 	offset, size := stack.pop(), stack.pop()
-	data := memory.Get(offset.Int64(), size.Int64())
+	data := memory.GetPtr(offset.Int64(), size.Int64())
 
 	if interpreter.hasher == nil {
 		interpreter.hasher = sha3.NewLegacyKeccak256().(keccakState)
@@ -757,7 +757,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract, memory
 	toAddr := common.BigToAddress(addr)
 	value = math.U256(value)
 	// Get the arguments from the memory.
-	args := memory.Get(inOffset.Int64(), inSize.Int64())
+	args := memory.GetPtr(inOffset.Int64(), inSize.Int64())
 
 	if value.Sign() != 0 {
 		gas += params.CallStipend
@@ -786,7 +786,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, contract *Contract, mem
 	toAddr := common.BigToAddress(addr)
 	value = math.U256(value)
 	// Get arguments from the memory.
-	args := memory.Get(inOffset.Int64(), inSize.Int64())
+	args := memory.GetPtr(inOffset.Int64(), inSize.Int64())
 
 	if value.Sign() != 0 {
 		gas += params.CallStipend
@@ -814,7 +814,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract,
 	addr, inOffset, inSize, retOffset, retSize := stack.pop(), stack.pop(), stack.pop(), stack.pop(), stack.pop()
 	toAddr := common.BigToAddress(addr)
 	// Get arguments from the memory.
-	args := memory.Get(inOffset.Int64(), inSize.Int64())
+	args := memory.GetPtr(inOffset.Int64(), inSize.Int64())
 
 	ret, returnGas, err := interpreter.evm.DelegateCall(contract, toAddr, args, gas)
 	if err != nil {
@@ -839,7 +839,7 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, contract *Contract, m
 	addr, inOffset, inSize, retOffset, retSize := stack.pop(), stack.pop(), stack.pop(), stack.pop(), stack.pop()
 	toAddr := common.BigToAddress(addr)
 	// Get arguments from the memory.
-	args := memory.Get(inOffset.Int64(), inSize.Int64())
+	args := memory.GetPtr(inOffset.Int64(), inSize.Int64())
 
 	ret, returnGas, err := interpreter.evm.StaticCall(contract, toAddr, args, gas)
 	if err != nil {