fix HashMessage to decode hex only if needed (#1450)

* hash message expects a string and not a hex string

* refactor HashMessage

* swap returned values

api/utils.go

@@ -1,7 +1,10 @@
 package api
 
 import (
-	"fmt"
+	"bytes"
+	"encoding/hex"
+	"strconv"
+	"strings"
 
 	"github.com/ethereum/go-ethereum/crypto"
 )
@@ -21,7 +24,36 @@ func RunAsync(f func() error) <-chan error {
 // The hash is calulcated as
 //   keccak256("\x19Ethereum Signed Message:\n"${message length}${message}).
 // This gives context to the signed message and prevents signing of transactions.
-func HashMessage(data []byte) []byte {
+func HashMessage(message string) ([]byte, error) {
-	msg := fmt.Sprintf("\x19Ethereum Signed Message:\n%d%s", len(data), data)
+	buf := bytes.NewBufferString("\x19Ethereum Signed Message:\n")
-	return crypto.Keccak256([]byte(msg))
+	if value, ok := decodeHexStrict(message); ok {
+		if _, err := buf.WriteString(strconv.Itoa(len(value))); err != nil {
+			return nil, err
+		}
+		if _, err := buf.Write(value); err != nil {
+			return nil, err
+		}
+	} else {
+		if _, err := buf.WriteString(strconv.Itoa(len(message))); err != nil {
+			return nil, err
+		}
+		if _, err := buf.WriteString(message); err != nil {
+			return nil, err
+		}
+	}
+
+	return crypto.Keccak256(buf.Bytes()), nil
+}
+
+func decodeHexStrict(s string) ([]byte, bool) {
+	if !strings.HasPrefix(s, "0x") {
+		return nil, false
+	}
+
+	value, err := hex.DecodeString(s[2:])
+	if err != nil {
+		return nil, false
+	}
+
+	return value, true
 }

api/utils_test.go

@@ -26,20 +26,53 @@ func TestHashMessage(t *testing.T) {
 	require.NoError(t, err)
 	addr := crypto.PubkeyToAddress(key.PublicKey)
 
-	originalMessage := []byte{0x01, 0x02, 0x03}
+	scenarios := []struct {
-	hash := HashMessage(originalMessage)
+		message        string
-
+		expectedHash   string
-	// simulate signature from external signer like a keycard
+		recoverMessage string
-	sig, err := crypto.Sign(hash, key)
+	}{
-	require.NoError(t, err)
+		{
-	sig[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
+			message:        "XYZ",
-
+			expectedHash:   "634349abf2de883d23e8b46972896c7652a06670c990410d3436d9b44db09e6b",
-	// check that the message was wrapped correctly before hashing it
+			recoverMessage: fmt.Sprintf("0x%x", "XYZ"),
-	recParams := personal.RecoverParams{
+		},
-		Message:   fmt.Sprintf("0x%x", originalMessage),
+		{
-		Signature: fmt.Sprintf("0x%x", sig),
+			message:        "0xXYZ",
+			expectedHash:   "f9c57a8998c71a2c8d74d70abe6561838f0d6cb6d82bc85bd70afcc82368055c",
+			recoverMessage: fmt.Sprintf("0x%x", "0xXYZ"),
+		},
+		{
+			message:        "1122",
+			expectedHash:   "3f07e02a153f02bdf97d77161746257626e9c39e4c3cf59896365fd1e6a9c7c3",
+			recoverMessage: fmt.Sprintf("0x%x", "1122"),
+		},
+		{
+			message:        "0x1122",
+			expectedHash:   "86d79d0957efa9b7d91f1116e70d0ee934cb9cdeccefa07756aed2bee119a2f3",
+			recoverMessage: "0x1122",
+		},
+	}
+
+	for _, s := range scenarios {
+		t.Run(s.message, func(t *testing.T) {
+			hash, err := HashMessage(s.message)
+			require.Nil(t, err)
+			require.Equal(t, s.expectedHash, fmt.Sprintf("%x", hash))
+
+			// simulate signature from external signer like a keycard
+			sig, err := crypto.Sign(hash, key)
+			require.NoError(t, err)
+			sig[64] += 27 // Transform V from 0/1 to 27/28 according to the yellow paper
+
+			// check that the message was wrapped correctly before hashing it
+			recParams := personal.RecoverParams{
+				Message:   s.recoverMessage,
+				Signature: fmt.Sprintf("0x%x", sig),
+			}
+
+			recoveredAddr, err := backend.Recover(recParams)
+			require.NoError(t, err)
+			assert.Equal(t, addr, recoveredAddr)
+		})
 	}
-	recoveredAddr, err := backend.Recover(recParams)
-	require.NoError(t, err)
-	assert.Equal(t, addr, recoveredAddr)
 }

lib/library.go

@@ -459,14 +459,13 @@ func HashTransaction(txArgsJSON *C.char) *C.char {
 //   keccak256("\x19Ethereum Signed Message:\n"${message length}${message}).
 // This gives context to the signed message and prevents signing of transactions.
 //export HashMessage
-func HashMessage(messageString *C.char) *C.char {
+func HashMessage(message *C.char) *C.char {
-	message, err := hex.DecodeString(C.GoString(messageString))
+	hash, err := api.HashMessage(C.GoString(message))
-	if err != nil {
+	code := codeUnknown
-		return C.CString(prepareJSONResponseWithCode(nil, err, codeFailedParseParams))
+	if c, ok := errToCodeMap[err]; ok {
+		code = c
 	}
-
+	return C.CString(prepareJSONResponseWithCode(fmt.Sprintf("0x%x", hash), err, code))
-	hash := api.HashMessage(message)
-	return C.CString(prepareJSONResponseWithCode(fmt.Sprintf("0x%x", hash), err, codeUnknown))
 }
 
 // SignTypedData unmarshall data into TypedData, validate it and signs with selected account,

mobile/status.go

@@ -460,14 +460,13 @@ func HashTransaction(txArgsJSON string) string {
 // The hash is calulcated as
 //   keccak256("\x19Ethereum Signed Message:\n"${message length}${message}).
 // This gives context to the signed message and prevents signing of transactions.
-func HashMessage(messageString string) string {
+func HashMessage(message string) string {
-	message, err := hex.DecodeString(messageString)
+	hash, err := api.HashMessage(message)
-	if err != nil {
+	code := codeUnknown
-		return prepareJSONResponseWithCode(nil, err, codeFailedParseParams)
+	if c, ok := errToCodeMap[err]; ok {
+		code = c
 	}
-
+	return prepareJSONResponseWithCode(fmt.Sprintf("0x%x", hash), err, code)
-	hash := api.HashMessage(message)
-	return prepareJSONResponseWithCode(fmt.Sprintf("0x%x", hash), err, codeUnknown)
 }
 
 // StartCPUProfile runs pprof for CPU.