diff --git a/cmd/clef/main.go b/cmd/clef/main.go index 02498cc22a..ad7ba186d0 100644 --- a/cmd/clef/main.go +++ b/cmd/clef/main.go @@ -671,7 +671,7 @@ func testExternalUI(api *core.SignerAPI) { } } var delay = 1 * time.Second - //Test display of info and error + // Test display of info and error { api.UI.ShowInfo("If you see this message, enter 'yes' to next question") time.Sleep(delay) diff --git a/signer/core/signed_data.go b/signer/core/signed_data.go index 9b0753e8b3..27eca91831 100644 --- a/signer/core/signed_data.go +++ b/signer/core/signed_data.go @@ -327,7 +327,10 @@ func (api *SignerAPI) SignTypedData(ctx context.Context, addr common.MixedcaseAd } rawData := []byte(fmt.Sprintf("\x19\x01%s%s", string(domainSeparator), string(typedDataHash))) sighash := crypto.Keccak256(rawData) - message := typedData.Format() + message, err := typedData.Format() + if err != nil { + return nil, err + } req := &SignDataRequest{ContentType: DataTyped.Mime, Rawdata: rawData, Message: message, Hash: sighash} signature, err := api.sign(addr, req, true) if err != nil { @@ -482,8 +485,12 @@ func (typedData *TypedData) EncodeData(primaryType string, data map[string]inter return buffer.Bytes(), nil } -func parseIntegerType(encType string, encValue interface{}) (*big.Int, error) { - length := 0 +func parseInteger(encType string, encValue interface{}) (*big.Int, error) { + var ( + length = 0 + signed = strings.HasPrefix(encType, "int") + b *big.Int + ) if encType == "int" || encType == "uint" { length = 256 } else { @@ -499,7 +506,6 @@ func parseIntegerType(encType string, encValue interface{}) (*big.Int, error) { } length = atoiSize } - var b *big.Int switch v := encValue.(type) { case *math.HexOrDecimal256: b = (*big.Int)(v) @@ -524,6 +530,9 @@ func parseIntegerType(encType string, encValue interface{}) (*big.Int, error) { if b.BitLen() > length { return nil, fmt.Errorf("integer larger than '%v'", encType) } + if !signed && b.Sign() == -1 { + return nil, fmt.Errorf("invalid negative value for unsigned type %v", encType) + } return b, nil } @@ -577,7 +586,7 @@ func (typedData *TypedData) EncodePrimitiveValue(encType string, encValue interf } } if strings.HasPrefix(encType, "int") || strings.HasPrefix(encType, "uint") { - b, err := parseIntegerType(encType, encValue) + b, err := parseInteger(encType, encValue) if err != nil { return nil, err } @@ -680,35 +689,32 @@ func (typedData *TypedData) Map() map[string]interface{} { return dataMap } -// PrettyPrint generates a nice output to help the users -// of clef present data in their apps -func (typedData *TypedData) PrettyPrint() string { - output := bytes.Buffer{} - formatted := typedData.Format() - for _, item := range formatted { - output.WriteString(fmt.Sprintf("%v\n", item.Pprint(0))) - } - return output.String() -} - // Format returns a representation of typedData, which can be easily displayed by a user-interface // without in-depth knowledge about 712 rules -func (typedData *TypedData) Format() []*NameValueType { +func (typedData *TypedData) Format() ([]*NameValueType, error) { + domain, err := typedData.formatData("EIP712Domain", typedData.Domain.Map()) + if err != nil { + return nil, err + } + ptype, err := typedData.formatData(typedData.PrimaryType, typedData.Message) + if err != nil { + return nil, err + } var nvts []*NameValueType nvts = append(nvts, &NameValueType{ Name: "EIP712Domain", - Value: typedData.formatData("EIP712Domain", typedData.Domain.Map()), + Value: domain, Typ: "domain", }) nvts = append(nvts, &NameValueType{ Name: typedData.PrimaryType, - Value: typedData.formatData(typedData.PrimaryType, typedData.Message), + Value: ptype, Typ: "primary type", }) - return nvts + return nvts, nil } -func (typedData *TypedData) formatData(primaryType string, data map[string]interface{}) []*NameValueType { +func (typedData *TypedData) formatData(primaryType string, data map[string]interface{}) ([]*NameValueType, error) { var output []*NameValueType // Add field contents. Structs and arrays have special handlers. @@ -725,50 +731,70 @@ func (typedData *TypedData) formatData(primaryType string, data map[string]inter for _, v := range arrayValue { if typedData.Types[parsedType] != nil { mapValue, _ := v.(map[string]interface{}) - mapOutput := typedData.formatData(parsedType, mapValue) + mapOutput, err := typedData.formatData(parsedType, mapValue) + if err != nil { + return nil, err + } item.Value = mapOutput } else { - primitiveOutput := formatPrimitiveValue(field.Type, encValue) + primitiveOutput, err := formatPrimitiveValue(field.Type, encValue) + if err != nil { + return nil, err + } item.Value = primitiveOutput } } } else if typedData.Types[field.Type] != nil { if mapValue, ok := encValue.(map[string]interface{}); ok { - mapOutput := typedData.formatData(field.Type, mapValue) + mapOutput, err := typedData.formatData(field.Type, mapValue) + if err != nil { + return nil, err + } item.Value = mapOutput } else { item.Value = "" } } else { - primitiveOutput := formatPrimitiveValue(field.Type, encValue) + primitiveOutput, err := formatPrimitiveValue(field.Type, encValue) + if err != nil { + return nil, err + } item.Value = primitiveOutput } output = append(output, item) } - return output + return output, nil } -func formatPrimitiveValue(encType string, encValue interface{}) string { +func formatPrimitiveValue(encType string, encValue interface{}) (string, error) { switch encType { case "address": - stringValue, _ := encValue.(string) - return common.HexToAddress(stringValue).String() + if stringValue, ok := encValue.(string); !ok { + return "", fmt.Errorf("could not format value %v as address", encValue) + } else { + return common.HexToAddress(stringValue).String(), nil + } case "bool": - boolValue, _ := encValue.(bool) - return fmt.Sprintf("%t", boolValue) + if boolValue, ok := encValue.(bool); !ok { + return "", fmt.Errorf("could not format value %v as bool", encValue) + } else { + return fmt.Sprintf("%t", boolValue), nil + } case "bytes", "string": - return fmt.Sprintf("%s", encValue) + return fmt.Sprintf("%s", encValue), nil } if strings.HasPrefix(encType, "bytes") { - return fmt.Sprintf("%s", encValue) - } else if strings.HasPrefix(encType, "uint") || strings.HasPrefix(encType, "int") { - b, err := parseIntegerType(encType, encValue) - if err != nil { - return fmt.Sprintf("ERROR: %v", err) - } - return fmt.Sprintf("%d (0x%x)", b, b) + return fmt.Sprintf("%s", encValue), nil + } - return "NA" + if strings.HasPrefix(encType, "uint") || strings.HasPrefix(encType, "int") { + if b, err := parseInteger(encType, encValue); err != nil { + return "", err + } else { + return fmt.Sprintf("%d (0x%x)", b, b), nil + } + } + return "", fmt.Errorf("unhandled type %v", encType) } // NameValueType is a very simple struct with Name, Value and Type. It's meant for simple diff --git a/signer/core/signed_data_internal_test.go b/signer/core/signed_data_internal_test.go new file mode 100644 index 0000000000..b81872566d --- /dev/null +++ b/signer/core/signed_data_internal_test.go @@ -0,0 +1,51 @@ +// Copyright 2019 The go-ethereum Authors +// This file is part of go-ethereum. +// +// go-ethereum is free software: you can redistribute it and/or modify +// it under the terms of the GNU General Public License as published by +// the Free Software Foundation, either version 3 of the License, or +// (at your option) any later version. +// +// go-ethereum is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. +// +// You should have received a copy of the GNU General Public License +// along with go-ethereum. If not, see . +// +package core + +import ( + "math/big" + "testing" +) + +func TestParseInteger(t *testing.T) { + for i, tt := range []struct { + t string + v interface{} + exp *big.Int + }{ + {"uint32", "-123", nil}, + {"int32", "-123", big.NewInt(-123)}, + {"uint32", "0xff", big.NewInt(0xff)}, + {"int8", "0xffff", nil}, + } { + res, err := parseInteger(tt.t, tt.v) + if tt.exp == nil && res == nil { + continue + } + if tt.exp == nil && res != nil { + t.Errorf("test %d, got %v, expected nil", i, res) + continue + } + if tt.exp != nil && res == nil { + t.Errorf("test %d, got '%v', expected %v", i, err, tt.exp) + continue + } + if tt.exp.Cmp(res) != 0 { + t.Errorf("test %d, got %v expected %v", i, res, tt.exp) + } + } +} diff --git a/signer/core/signed_data_test.go b/signer/core/signed_data_test.go index 6f0e53e12d..5a18defaac 100644 --- a/signer/core/signed_data_test.go +++ b/signer/core/signed_data_test.go @@ -319,7 +319,7 @@ func TestFormatter(t *testing.T) { if err != nil { t.Fatalf("unmarshalling failed '%v'", err) } - formatted := d.Format() + formatted, _ := d.Format() for _, item := range formatted { fmt.Printf("'%v'\n", item.Pprint(0)) } @@ -403,7 +403,6 @@ func TestFuzzerFiles(t *testing.T) { if verbose && err != nil { fmt.Printf("%d, EncodeData[2] err: %v\n", i, err) } - typedData.PrettyPrint() typedData.Format() } }