From b5ada7ea70326a2e7a09b38a8207edb415feaa35 Mon Sep 17 00:00:00 2001 From: Paul Berg Date: Wed, 14 Nov 2018 00:25:18 +0200 Subject: [PATCH] Fixed more new failing tests and deanonymised some functions --- signer/core/signed_data.go | 173 ++++++------- signer/core/signed_data_test.go | 430 +++++++++++++++++--------------- 2 files changed, 317 insertions(+), 286 deletions(-) diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 68d3576845..5cb2b3ebe8 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -365,92 +365,28 @@ func (typedData *TypedData) TypeHash(primaryType string) hexutil.Bytes { // // each encoded member is 32-byte long func (typedData *TypedData) EncodeData(primaryType string, data map[string]interface{}) (hexutil.Bytes, error) { - encTypes := []string{} encValues := []interface{}{} + // Verify extra data + if len(typedData.Types[primaryType]) < len(data) { + return nil, errors.New("there is extra data provided in the message") + } + // Add typehash - encTypes = append(encTypes, "bytes32") encValues = append(encValues, typedData.TypeHash(primaryType)) - // Generate error for a mismatch between the provided type and data - dataMismatchError := func(encType string, encValue interface{}) error { - return fmt.Errorf("provided data '%v' doesn't match type '%s'", encValue, encType) - } - - // Handle primitive values - handlePrimitiveValue := func(encType string, encValue interface{}) (string, interface{}, error) { - var primitiveEncType string - var primitiveEncValue interface{} - - switch encType { - case "address": - primitiveEncType = "uint160" - bytesValue := hexutil.Bytes{} - for i := 0; i < 12; i++ { - bytesValue = append(bytesValue, 0) - } - stringValue, ok := encValue.(string) - if !ok || !common.IsHexAddress(stringValue) { - return "", nil, dataMismatchError(encType, encValue) - } - for _, _byte := range common.HexToAddress(stringValue) { - bytesValue = append(bytesValue, _byte) - } - primitiveEncValue = bytesValue - case "bool": - primitiveEncType = "uint256" - var int64Val int64 - boolValue, ok := encValue.(bool) - if !ok { - return "", nil, dataMismatchError(encType, encValue) - } - if boolValue { - int64Val = 1 - } - primitiveEncValue = abi.U256(big.NewInt(int64Val)) - case "bytes", "string": - primitiveEncType = "bytes32" - bytesValue, err := bytesValueOf(encValue) - if err != nil { - return "", nil, dataMismatchError(encType, encValue) - } - primitiveEncValue = crypto.Keccak256(bytesValue) - default: - if strings.HasPrefix(encType, "bytes") { - encTypes = append(encTypes, "bytes32") - sizeStr := strings.TrimPrefix(encType, "bytes") - size, _ := strconv.Atoi(sizeStr) - bytesValue := hexutil.Bytes{} - for i := 0; i < 32-size; i++ { - bytesValue = append(bytesValue, 0) - } - if _, ok := encValue.(hexutil.Bytes); !ok { - return "", nil, dataMismatchError(encType, encValue) - } - bytesValue = append(bytesValue, encValue.(hexutil.Bytes)...) - primitiveEncValue = bytesValue - } else if strings.HasPrefix(encType, "uint") || strings.HasPrefix(encType, "int") { - primitiveEncType = "uint256" - bigIntValue, ok := encValue.(*big.Int) - if !ok { - return "", nil, dataMismatchError(encType, encValue) - } - primitiveEncValue = abi.U256(bigIntValue) - } - } - return primitiveEncType, primitiveEncValue, nil - } - - // Add field contents. Structs and arrays have special handlings. + // Add field contents. Structs and arrays have special handlers. for _, field := range typedData.Types[primaryType] { encType := field["type"] encValue := data[field["name"]] if encType[len(encType)-1:] == "]" { - encTypes = append(encTypes, "bytes32") + arrayValue, ok := encValue.([]interface{}) + if !ok { + return nil, dataMismatchError(encType, encValue) + } parsedType := strings.Split(encType, "[")[0] - arrayBuffer := bytes.Buffer{} - for _, item := range encValue.([]interface{}) { + for _, item := range arrayValue { if typedData.Types[parsedType] != nil { mapValue, ok := item.(map[string]interface{}) if !ok { @@ -462,7 +398,7 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter } arrayBuffer.Write(encodedData) } else { - _, encValue, err := handlePrimitiveValue(encType, encValue) + encValue, err := handlePrimitiveValue(encType, encValue) if err != nil { return nil, err } @@ -475,7 +411,6 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter } encValues = append(encValues, crypto.Keccak256(arrayBuffer.Bytes())) } else if typedData.Types[field["type"]] != nil { - encTypes = append(encTypes, "bytes32") mapValue, ok := encValue.(map[string]interface{}) if !ok { return nil, dataMismatchError(encType, encValue) @@ -487,11 +422,10 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter encValue = crypto.Keccak256(encodedData) encValues = append(encValues, encValue) } else { - primitiveEncType, primitiveEncValue, err := handlePrimitiveValue(encType, encValue) + primitiveEncValue, err := handlePrimitiveValue(encType, encValue) if err != nil { return nil, err } - encTypes = append(encTypes, primitiveEncType) encValues = append(encValues, primitiveEncValue) } } @@ -508,6 +442,72 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter return buffer.Bytes(), nil // https://github.com/ethereumjs/ethereumjs-abi/blob/master/lib/index.js#L336 } +// handlePrimitiveValues deals with the primitive values found +// while searching through the typed data +func handlePrimitiveValue(encType string, encValue interface{}) (interface{}, error) { + var primitiveEncValue interface{} + + switch encType { + case "address": + bytesValue := hexutil.Bytes{} + for i := 0; i < 12; i++ { + bytesValue = append(bytesValue, 0) + } + stringValue, ok := encValue.(string) + if !ok || !common.IsHexAddress(stringValue) { + return nil, dataMismatchError(encType, encValue) + } + for _, _byte := range common.HexToAddress(stringValue) { + bytesValue = append(bytesValue, _byte) + } + primitiveEncValue = bytesValue + case "bool": + var int64Val int64 + boolValue, ok := encValue.(bool) + if !ok { + return nil, dataMismatchError(encType, encValue) + } + if boolValue { + int64Val = 1 + } + primitiveEncValue = abi.U256(big.NewInt(int64Val)) + case "bytes", "string": + bytesValue, err := bytesValueOf(encValue) + if err != nil { + return nil, dataMismatchError(encType, encValue) + } + primitiveEncValue = crypto.Keccak256(bytesValue) + default: + if strings.HasPrefix(encType, "bytes") { + sizeStr := strings.TrimPrefix(encType, "bytes") + size, _ := strconv.Atoi(sizeStr) + bytesValue := hexutil.Bytes{} + for i := 0; i < 32-size; i++ { + bytesValue = append(bytesValue, 0) + } + if _, ok := encValue.(hexutil.Bytes); !ok { + return nil, dataMismatchError(encType, encValue) + } + bytesValue = append(bytesValue, encValue.(hexutil.Bytes)...) + primitiveEncValue = bytesValue + } else if strings.HasPrefix(encType, "uint") || strings.HasPrefix(encType, "int") { + bigIntValue, ok := encValue.(*big.Int) + if !ok { + return nil, dataMismatchError(encType, encValue) + } + primitiveEncValue = abi.U256(bigIntValue) + } + } + return primitiveEncValue, nil +} + +// dataMismatchError generates an error for a mismatch between +// the provided type and data +func dataMismatchError(encType string, encValue interface{}) error { + return fmt.Errorf("provided data '%v' doesn't match type '%s'", encValue, encType) +} + +// bytesValuesOf returns the bytes value of the given interface func bytesValueOf(_interface interface{}) (hexutil.Bytes, error) { bytesValue, ok := _interface.(hexutil.Bytes) if ok { @@ -585,7 +585,7 @@ func UnmarshalValidatorData(data interface{}) (ValidatorData, error) { }, nil } -// Validate checks if the typed data is sound +// Validate make sure the types are sound func (typedData *TypedData) Validate() error { if err := typedData.Types.Validate(); err != nil { return err @@ -604,15 +604,9 @@ func (typedData *TypedData) Map() map[string]interface{} { "primaryType": typedData.PrimaryType, "message": typedData.Message, } - return dataMap } -// PrettyPrint generates a pretty version of the typed data -func (typedData *TypedData) PrettyPrint() string { - return "" -} - // Validate checks if the types object is conformant to the specs func (types *EIP712Types) Validate() error { for typeKey, typeArr := range *types { @@ -644,19 +638,18 @@ func (types *EIP712Types) Validate() error { func isStandardTypeStr(encType string) bool { // Atomic types exp, _ := regexp.Compile(`^(address|bool|bytes|string)$`) - if (exp.MatchString(encType)) { + if exp.MatchString(encType) { return true } // Dynamic types exp, _ = regexp.Compile(`^(bytes|int|uint)(\d+)$`) - if (exp.MatchString(encType)) { + if exp.MatchString(encType) { return true } // Arrays - // TODO: add dynamic type arrays - exp, _ = regexp.Compile(`^(address|bool|bytes|string)\[]$`) + exp, _ = regexp.Compile(`^(address|bool|bytes|string|((bytes|int|uint)(\d+)))\[]$`) return exp.MatchString(encType) } diff --git a/signer/core/signed_data_test.go b/signer/core/signed_data_test.go index 697fd0e35e..5e130db1f4 100644 --- a/signer/core/signed_data_test.go +++ b/signer/core/signed_data_test.go @@ -204,171 +204,14 @@ func TestEncodeData(t *testing.T) { } func TestMalformedData1(t *testing.T) { - var data = ` - { - "types": { - "EIP712Domain": [ - { - "name": "name", - "type": "string" - }, - { - "name": "version", - "type": "string" - }, - { - "name": "chainId", - "type": "uint256" - }, - { - "name": "verifyingContract", - "type": "address" - } - ], - "Person": [ - { - "name": "name", - "type": "string" - }, - { - "name": "wallet", - "type": "address" - } - ], - "Mail": [ - { - "name": "from", - "type": "Person" - }, - { - "name": "to", - "type": "Person" - }, - { - "name": "contents", - "type": "Person" - } - ] - }, - "primaryType": "Mail", - "domain": { - "name": "Ether Mail", - "version": "1", - "chainId": 1, - "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" - }, - "message": { - "from": { - "name": "Cow", - "wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" - }, - "to": { - "name": "Bob", - "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" - }, - "contents": "Hello, Bob!" - } - } - -` - var typedData TypedData - err := json.Unmarshal([]byte(data), &typedData) - if err != nil { - t.Fatalf("unmarshalling failed %v", err) - } - err = typedData.Validate() - if err != nil { - t.Fatalf("Expected no error, got %v", err) - } - _, err = typedData.HashStruct(typedData.PrimaryType, typedData.Message) - if err.Error() != "provided data 'Hello, Bob!' doesn't match type 'Person'" { - t.Errorf("Expected `provided data 'Hello, Bob!' doesn't match type 'Person'`, got %v", err) - } -} - -func TestMalformedDomainData(t *testing.T) { - var data = ` -{ - "types": { - "EIP712Domain": [ - { - "name": "name", - "type": "string" - }, - { - "name": "version", - "type": "string" - }, - { - "name": "chainId", - "type": "uint256" - }, - { - "name": "verifyingContract", - "type": "address" - } - ], - "Person": [ - { - "name": "name", - "type": "string" - }, - { - "name": "wallet", - "type": "address" - } - ], - "Mail": [ - { - "name": "from", - "type": "Person" - }, - { - "name": "to", - "type": "Person" - }, - { - "name": "contents", - "type": "Blahonga" - } - ] - }, - "primaryType": "Mail", - "domain": { - "name": "Ether Mail", - "version": "1", - "chainId": 1, - "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" - }, - "message": { - "from": { - "name": "Cow", - "wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" - }, - "to": { - "name": "Bob", - "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" - }, - "contents": "Hello, Bob!" - } - }` - var typedData TypedData - err := json.Unmarshal([]byte(data), &typedData) - if err != nil { - t.Fatalf("unmarshalling failed %v", err) - } - err = typedData.Validate() - if err == nil { - t.Fatalf("Expected `referenced type 'Blahonga' is undefined`, got %v", err) - } - _, err = typedData.HashStruct(typedData.PrimaryType, typedData.Message) - if err.Error() != "unrecognized interface type " { - t.Errorf("Expected `unrecognized interface type `, got %v", err) - } -} - -func TestMalformedData3(t *testing.T) { - var data = ` + // Verifies that malformed domain keys are properly caught: + //{ + // "name": "Ether Mail", + // "version": "1", + // "chainId": 1, + // "vxerifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" + //} + var jsonTypedData = ` { "types": { "EIP712Domain": [ @@ -435,23 +278,140 @@ func TestMalformedData3(t *testing.T) { } ` - var typedData TypedData - err := json.Unmarshal([]byte(data), &typedData) + var malformedTypedData TypedData + err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData) if err != nil { t.Fatalf("unmarshalling failed %v", err) } - err = typedData.Validate() + err = malformedTypedData.Validate() if err != nil { t.Fatalf("Expected no error, got %v", err) } - _, err = typedData.HashStruct("EIP712Domain", typedData.Domain.Map()) - if err.Error() != "provided data '' doesn't match type 'address'" { + _, err = malformedTypedData.HashStruct("EIP712Domain", malformedTypedData.Domain.Map()) + if err == nil || err.Error() != "provided data '' doesn't match type 'address'" { t.Errorf("Expected `provided data '' doesn't match type 'address'`, got %v", err) } } -func TestMalformedData4(t *testing.T) { - var data = ` +func TestMalformedData2(t *testing.T) { + // Verifies that: + // 1. Mismatches between the given type and data, i.e. `Person` and + // the data item is a string, are properly caught: + //{ + // "name": "contents", + // "type": "Person" + //}, + //{ + // "contents": "Hello, Bob!" <-- string not "Person" + //} + // 2. Nonexistent types are properly caught: + //{ + // "name": "contents", + // "type": "Blahonga" + //} + jsonTypedData := ` + { + "types": { + "EIP712Domain": [ + { + "name": "name", + "type": "string" + }, + { + "name": "version", + "type": "string" + }, + { + "name": "chainId", + "type": "uint256" + }, + { + "name": "verifyingContract", + "type": "address" + } + ], + "Person": [ + { + "name": "name", + "type": "string" + }, + { + "name": "wallet", + "type": "address" + } + ], + "Mail": [ + { + "name": "from", + "type": "Person" + }, + { + "name": "to", + "type": "Person" + }, + { + "name": "contents", + "type": "Person" + } + ] + }, + "primaryType": "Mail", + "domain": { + "name": "Ether Mail", + "version": "1", + "chainId": 1, + "verifyingContract": "0xCcCCccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" + }, + "message": { + "from": { + "name": "Cow", + "wallet": "0xCD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" + }, + "to": { + "name": "Bob", + "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" + }, + "contents": "Hello, Bob!" + } + } +` + var malformedTypedData TypedData + err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData) + if err != nil { + t.Fatalf("unmarshalling failed %v", err) + } + err = malformedTypedData.Validate() + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + _, err = malformedTypedData.HashStruct(malformedTypedData.PrimaryType, malformedTypedData.Message) + if err.Error() != "provided data 'Hello, Bob!' doesn't match type 'Person'" { + t.Errorf("Expected `provided data 'Hello, Bob!' doesn't match type 'Person'`, got %v", err) + } + + malformedTypedData.Types["Mail"][2]["type"] = "Blahonga" + err = malformedTypedData.Validate() + if err == nil || err.Error() != "referenced type 'Blahonga' is undefined" { + t.Fatalf("Expected `referenced type 'Blahonga' is undefined`, got %v", err) + } + _, err = malformedTypedData.HashStruct(malformedTypedData.PrimaryType, malformedTypedData.Message) + if err == nil || err.Error() != "unrecognized interface type " { + t.Errorf("Expected `unrecognized interface type `, got %v", err) + } +} + +func TestMalformedData3(t *testing.T) { + // Verifies several quirks + // 1. Using dynamic types and only validating the prefix: + //{ + // "name": "chainId", + // "type": "uint256 ... and now for something completely different" + //} + // 2. Extra data in message: + //{ + // "blahonga": "zonk bonk" + //} + jsonTypedData := ` { "types": { "EIP712Domain": [ @@ -472,6 +432,95 @@ func TestMalformedData4(t *testing.T) { "type": "address" } ], + "Person": [ + { + "name": "name", + "type": "string" + }, + { + "name": "wallet", + "type": "address" + } + ], + "Mail": [ + { + "name": "from", + "type": "Person" + }, + { + "name": "to", + "type": "Person" + }, + { + "name": "contents", + "type": "string" + } + ] + }, + "primaryType": "Mail", + "domain": { + "name": "Ether Mail", + "version": "1", + "chainId": 1, + "verifyingContract": "0xCCCcccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" + }, + "message": { + "from": { + "name": "Cow", + "wallet": "0xcD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" + }, + "to": { + "name": "Bob", + "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" + }, + "contents": "Hello, Bob!" + } + } +` + var malformedTypedData TypedData + err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData) + if err != nil { + t.Fatalf("unmarshalling failed %v", err) + } + err = malformedTypedData.Validate() + if err == nil || err.Error() != "unknown atomic type 'uint256 ... and now for something completely different'" { + t.Fatalf("Expected `unknown atomic type 'uint256 ... and now for something completely different'`, got %v", err) + } + + malformedTypedData.Types["EIP712Domain"][2]["type"] = "uint256" + malformedTypedData.Message["blahonga"] = "zonk bonk" + _, err = malformedTypedData.HashStruct(malformedTypedData.PrimaryType, malformedTypedData.Message) + if err == nil || err.Error() != "there is extra data provided in the message" { + t.Errorf("Expected `there is extra data provided in the message`, got %v", err) + } +} + +func TestMalformedData4(t *testing.T) { + // Verifies data that doesn't fit into it: + //{ + // "test": 65536 <-- test defined as uint8 + //} + jsonTypedData := ` + { + "types": { + "EIP712Domain": [ + { + "name": "name", + "type": "string" + }, + { + "name": "version", + "type": "string" + }, + { + "name": "chainId", + "type": "uint256" + }, + { + "name": "verifyingContract", + "type": "address" + } + ], "Person": [ { "name": "name", @@ -503,48 +552,37 @@ func TestMalformedData4(t *testing.T) { }, "primaryType": "Mail", "domain": { - "Signed by": "Bill Gates -- this text won't affect the hash'", - "we can": "stuff anything here, really", "name": "Ether Mail", - "version": "65536", + "version": "1", "chainId": 1, "verifyingContract": "0xCCCcccccCCCCcCCCCCCcCcCccCcCCCcCcccccccC" }, "message": { "from": { "name": "Cow", + "test": 65536, "wallet": "0xcD2a3d9F938E13CD947Ec05AbC7FE734Df8DD826" }, "to": { "name": "Bob", - "test": 65536, "wallet": "0xbBbBBBBbbBBBbbbBbbBbbbbBBbBbbbbBbBbbBBbB" }, - "blahonga": "zonk bonk", - "contents": "åäzö \r\n test, Bob!" + "contents": "Hello, Bob!" } } ` - // The struct above contains several quirks - // 1. Using dynamic types and only validating the prefix: - //{ - // "name": "chainId", - // "type": "uint256 ... and now for something completely different" - //}, - // 2. Using dynamic types, but not verifying that the data fits into it - // "test": 65536, <-- test defined as uint8 - // 3a. Extra data in message - // "blahonga": "zonk bonk", - // 3b ... and in domain - // "Signed by": "Bill Gates", - - var typedData TypedData - err := json.Unmarshal([]byte(data), &typedData) + var malformedTypedData TypedData + err := json.Unmarshal([]byte(jsonTypedData), &malformedTypedData) if err != nil { t.Fatalf("unmarshalling failed %v", err) } - err = typedData.Validate() - if err.Error() != "unknown atomic type 'uint256 ... and now for something completely different'" { - t.Fatalf("Expected `unknown atomic type 'uint256 ... and now for something completely different'`, got %v", err) + err = malformedTypedData.Validate() + if err != nil { + t.Fatalf("Expected no error, got %v", err) + } + + _, err = malformedTypedData.HashStruct(malformedTypedData.PrimaryType, malformedTypedData.Message) + if err == nil || err.Error() != "provided data '65536' doesn't match type 'uint8'" { + t.Fatalf("Expected `provided data '65536' doesn't match type 'uint8'`, got %v", err) } }