LaconicNetwork/cosmos-sdk
7
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 2 Wiki Activity

chore: signmode textual audit fixes (#15715)

Browse Source 
Co-authored-by: Amaury <1293565+amaurym@users.noreply.github.com>
Co-authored-by: 0000 1000 1101 0010 <96826920+08d2@users.noreply.github.com>
Co-authored-by: Aleksandr Bezobchuk <alexanderbez@users.noreply.github.com>
This commit is contained in:
Facundo Medica 2023-04-23 17:49:32 -03:00 committed by GitHub
parent 93d64cc6fe
commit f0018246f1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
16 changed files with 185 additions and 52 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

15
x/tx/signing/textual/any.go
View File

@ -52,10 +52,10 @@ func (ar anyValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]
// The Any value renderer suppresses emission of the object header for all
// messages that go through the messageValueRenderer.
omitHeader := 0
_, isMsgRenderer := vr.(*messageValueRenderer)
msgValRenderer, isMsgRenderer := vr.(*messageValueRenderer)
if isMsgRenderer {
if subscreens[0].Content != fmt.Sprintf("%s object", internalMsg.ProtoReflect().Descriptor().Name()) {
return nil, fmt.Errorf("any internal message expects %s, got %s", fmt.Sprintf("%s object", internalMsg.ProtoReflect().Descriptor().Name()), subscreens[0].Content)
if subscreens[0].Content != msgValRenderer.header() {
return nil, fmt.Errorf("any internal message expects %s, got %s", msgValRenderer.header(), subscreens[0].Content)
}
omitHeader = 1
@ -112,15 +112,16 @@ func (ar anyValueRenderer) Parse(ctx context.Context, screens []Screen) (protore
subscreens[i-1].Indent--
}
// Prepend with a "%s object" if the message goes through the default
// messageValueRenderer, and add a level of indentation.
_, isMsgRenderer := vr.(*messageValueRenderer)
// Append with "%s object" if the message goes through the default
// messageValueRenderer (the header() method does this for us), and
// add a level of indentation.
msgValRenderer, isMsgRenderer := vr.(*messageValueRenderer)
if isMsgRenderer {
for i := range subscreens {
subscreens[i].Indent++
}
subscreens = append([]Screen{{Content: fmt.Sprintf("%s object", msgType.Descriptor().Name())}}, subscreens...)
subscreens = append([]Screen{{Content: msgValRenderer.header()}}, subscreens...)
}
internalMsg, err := vr.Parse(ctx, subscreens)

4
x/tx/signing/textual/bytes.go
View File

@ -24,9 +24,7 @@ func NewBytesValueRenderer() ValueRenderer {
type bytesValueRenderer struct{}
var _ ValueRenderer = bytesValueRenderer{}
func (vr bytesValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]Screen, error) {
func (vr bytesValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) {
bz := v.Bytes()
if len(bz) <= maxByteLen {

8
x/tx/signing/textual/bytes_test.go
View File

@ -14,7 +14,7 @@ import (
func TestBytesJSONTestCases(t *testing.T) {
var testcases []bytesTest
// Bytes.json contains bytes that are represented in base64 format, and
// bytes.json contains bytes that are represented in base64 format, and
// their expected results in hex.
raw, err := os.ReadFile("./internal/testdata/bytes.json")
require.NoError(t, err)
@ -26,16 +26,16 @@ func TestBytesJSONTestCases(t *testing.T) {
for _, tc := range testcases {
t.Run(tc.hex, func(t *testing.T) {
valrend, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("BYTES"))
vr, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("BYTES"))
require.NoError(t, err)
screens, err := valrend.Format(context.Background(), protoreflect.ValueOfBytes(tc.base64))
screens, err := vr.Format(context.Background(), protoreflect.ValueOfBytes(tc.base64))
require.NoError(t, err)
require.Equal(t, 1, len(screens))
require.Equal(t, tc.hex, screens[0].Content)
// Round trip
val, err := valrend.Parse(context.Background(), screens)
val, err := vr.Parse(context.Background(), screens)
require.NoError(t, err)
if len(tc.base64) > 35 {
require.Equal(t, 0, len(val.Bytes()))

2
x/tx/signing/textual/coins.go
View File

@ -28,7 +28,7 @@ type coinsValueRenderer struct {
coinMetadataQuerier CoinMetadataQueryFn
}
var _ ValueRenderer = coinsValueRenderer{}
var _ RepeatedValueRenderer = coinsValueRenderer{}
func (vr coinsValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]Screen, error) {
if vr.coinMetadataQuerier == nil {

2
x/tx/signing/textual/dec.go
View File

@ -19,8 +19,6 @@ func NewDecValueRenderer() ValueRenderer {
type decValueRenderer struct{}
var _ ValueRenderer = decValueRenderer{}
func (vr decValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) {
decStr := v.String()

20
x/tx/signing/textual/duration.go
View File

@ -123,50 +123,50 @@ var durRegexp = regexp.MustCompile(`^(-)?(?:([0-9]+) days?)?(?:, )?(?:([0-9]+) h
// Parse implements the ValueRenderer interface.
func (dr durationValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
if len(screens) != 1 {
return protoreflect.Value{}, fmt.Errorf("expected single screen: %v", screens)
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}
parts := durRegexp.FindStringSubmatch(screens[0].Content)
if parts == nil {
return protoreflect.Value{}, fmt.Errorf("bad duration format: %s", screens[0].Content)
return nilValue, fmt.Errorf("bad duration format: %s", screens[0].Content)
}
negative := parts[1] != ""
isNegative := parts[1] == "-"
var days, hours, minutes, seconds, nanos int64
var err error
if parts[2] != "" {
days, err = strconv.ParseInt(parts[2], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[2], err)
return nilValue, fmt.Errorf(`bad number "%s": %w`, parts[2], err)
}
}
if parts[3] != "" {
hours, err = strconv.ParseInt(parts[3], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[3], err)
return nilValue, fmt.Errorf(`bad number "%s": %w`, parts[3], err)
}
}
if parts[4] != "" {
minutes, err = strconv.ParseInt(parts[4], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[4], err)
return nilValue, fmt.Errorf(`bad number "%s": %w`, parts[4], err)
}
}
if parts[5] != "" {
seconds, err = strconv.ParseInt(parts[5], 10, 64)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, parts[5], err)
return nilValue, fmt.Errorf(`bad number "%s": %w`, parts[5], err)
}
if parts[6] != "" {
if len(parts[6]) > 9 {
return protoreflect.Value{}, fmt.Errorf(`too many nanos "%s"`, parts[6])
return nilValue, fmt.Errorf(`too many nanos "%s"`, parts[6])
}
addZeros := 9 - len(parts[6])
text := parts[6] + strings.Repeat("0", addZeros)
nanos, err = strconv.ParseInt(text, 10, 32)
if err != nil {
return protoreflect.Value{}, fmt.Errorf(`bad number "%s": %w`, text, err)
return nilValue, fmt.Errorf(`bad number "%s": %w`, text, err)
}
}
}
@ -177,7 +177,7 @@ func (dr durationValueRenderer) Parse(_ context.Context, screens []Screen) (prot
// Since there are 9 digits or fewer, this conversion is safe.
dur.Nanos = int32(nanos)
if negative {
if isNegative {
dur.Seconds *= -1
dur.Nanos *= -1
}

2
x/tx/signing/textual/enum.go
View File

@ -20,8 +20,6 @@ func NewEnumValueRenderer(fd protoreflect.FieldDescriptor) ValueRenderer {
return enumValueRenderer{ed: ed}
}
var _ ValueRenderer = (*enumValueRenderer)(nil)
func (er enumValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) {
// Get the full name of the enum variant.
evd := er.ed.Values().ByNumber(v.Enum())

10
x/tx/signing/textual/handler.go
View File

@ -190,7 +190,8 @@ func (r *SignModeHandler) DefineMessageRenderer(name protoreflect.FullName, vr V
r.messages[name] = vr
}
// GetSignBytes returns the transaction sign bytes.
// GetSignBytes returns the transaction sign bytes which is the CBOR representation
// of a list of screens created from the TX data.
func (r *SignModeHandler) GetSignBytes(ctx context.Context, signerData signing.SignerData, txData signing.TxData) ([]byte, error) {
data := &textualpb.TextualData{
BodyBytes: txData.BodyBytes,
@ -204,12 +205,7 @@ func (r *SignModeHandler) GetSignBytes(ctx context.Context, signerData signing.S
},
}
vr, err := r.GetMessageValueRenderer(data.ProtoReflect().Descriptor())
if err != nil {
return nil, err
}
screens, err := vr.Format(ctx, protoreflect.ValueOf(data.ProtoReflect()))
screens, err := NewTxValueRenderer(r).Format(ctx, protoreflect.ValueOf(data.ProtoReflect()))
if err != nil {
return nil, err
}

2
x/tx/signing/textual/int.go
View File

@ -21,8 +21,6 @@ type intValueRenderer struct {
fd protoreflect.FieldDescriptor
}
var _ ValueRenderer = intValueRenderer{}
func (vr intValueRenderer) Format(_ context.Context, v protoreflect.Value) ([]Screen, error) {
formatted, err := math.FormatInt(v.String())
if err != nil {

18
x/tx/signing/textual/int_test.go
View File

@ -46,6 +46,24 @@ func TestIntJSONTestcases(t *testing.T) {
checkNumberTest(t, r, protoreflect.ValueOf(i), tc[1])
}
// Parse test case strings as protobuf int64
ii, err := strconv.ParseInt(tc[0], 10, 64)
if err == nil {
r, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("INT64"))
require.NoError(t, err)
checkNumberTest(t, r, protoreflect.ValueOf(ii), tc[1])
}
// Parse test case strings as protobuf int32
ii, err = strconv.ParseInt(tc[0], 10, 32)
if err == nil {
r, err := textual.GetFieldValueRenderer(fieldDescriptorFromName("INT32"))
require.NoError(t, err)
checkNumberTest(t, r, protoreflect.ValueOf(ii), tc[1])
}
// Parse test case strings as sdk.Ints
_, ok := math.NewIntFromString(tc[0])
if ok {

5
x/tx/signing/textual/internal/cbor/cbor.go
View File

@ -1,5 +1,6 @@
// Package cbor implements just enough of the CBOR (Concise Binary Object
// Representation, RFC 8948) to deterministically encode simple data.
// Representation, RFC 8948) to deterministically encode simple data. It does
// not include decoding as it is not needed for the purpose of this package.
package cbor
import (
@ -156,7 +157,7 @@ func NewMap(entries ...Entry) Map {
return Map{entries: entries}
}
// Add adds a key/value entry to an existimg Map.
// Add adds a key/value entry to an existing Map.
// Duplicate keys in the Map will cause an error when Encode is called.
func (m Map) Add(key, val Cbor) Map {
m.entries = append(m.entries, NewEntry(key, val))

126
x/tx/signing/textual/internal/testdata/e2e.json vendored
View File

@ -66,6 +66,132 @@
],
"cbor": "a1018fa20168436861696e20696402686d792d636861696ea2016e4163636f756e74206e756d626572026131a2016853657175656e6365026132a301674164647265737302782d636f736d6f7331756c6176336873656e7570737771666b77327933737570356b677471776e767161386579687304f5a3016a5075626c6963206b657902781f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657904f5a401634b657902785230324542204444374620453446442045423736204443384120323035452046363544203739304320443330452038413337203541354320323532382045423341203932334120463146422034443739203444030104f5a102781e54686973207472616e73616374696f6e206861732031204d657373616765a3016d4d6573736167652028312f312902781c2f636f736d6f732e62616e6b2e763162657461312e4d736753656e640301a3016c46726f6d206164647265737302782d636f736d6f7331756c6176336873656e7570737771666b77327933737570356b677471776e76716138657968730302a3016a546f206164647265737302782d636f736d6f7331656a726634637572327779366b667572673966326a707070326833616665356836706b6835740302a30166416d6f756e74026731302041544f4d0302a1026e456e64206f66204d657373616765a2016446656573026a302e3030322041544f4da30169476173206c696d697402673130302730303004f5a3017148617368206f66207261772062797465730278403738356264333036656138393632636462393630303038396264643635663364633032396531616561313132646565363965313935343663396164616438366504f5"
},
{
"name": "minimal with hashed bytes",
"proto": {
"body": {
"messages": [
{
"@type": "/A",
"BYTES": "0x12312312312312312312312312312312312312312312312122112223233124331243412351253126536123"
}
]
},
"auth_info": {
"signer_infos": [
{
"public_key": {
"@type": "/cosmos.crypto.secp256k1.PubKey",
"key": "Auvdf+T963bciiBe9l15DNMOijdaXCUo6zqSOvH7TXlN"
},
"mode_info": { "single": { "mode": "SIGN_MODE_TEXTUAL" } },
"sequence": 2
}
],
"fee": {
"amount": [{ "denom": "uatom", "amount": "2000" }],
"gas_limit": 100000
}
}
},
"signer_data": {
"account_number": 1,
"address": "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs",
"chain_id": "my-chain",
"pub_key": {
"@type": "/cosmos.crypto.secp256k1.PubKey",
"key": "Auvdf+T963bciiBe9l15DNMOijdaXCUo6zqSOvH7TXlN"
},
"sequence": 2
},
"metadata": {
"display": "ATOM",
"base": "uatom",
"denom_units": [
{ "denom": "ATOM", "exponent": 6 },
{ "denom": "uatom", "exponent": 0 }
]
},
"screens": [
{ "title": "Chain id", "content": "my-chain" },
{ "title": "Account number", "content": "1" },
{ "title": "Sequence", "content": "2" },
{ "title": "Address", "content": "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs", "expert": true },
{ "title": "Public key", "content": "/cosmos.crypto.secp256k1.PubKey", "expert": true },
{ "title": "Key", "content": "02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 1, "expert": true },
{ "content": "This transaction has 1 Message" },
{ "title": "Message (1/1)", "content": "/A", "indent": 1 },
{ "title": "BYTES", "content": "SHA-256=32BA 545C D070 3E09 0FFC D80F 20E7 1729 9D12 5D46 3728 8871 2B2D B2D7 CFD2 AA80", "indent": 2 },
{ "content": "End of Message" },
{ "title": "Fees", "content": "0.002 ATOM" },
{ "title": "Gas limit", "content": "100'000", "expert": true },
{ "title": "Hash of raw bytes", "content": "04241fbfa336b82b7fa9d3ad5d8706891798aa9a4978da9e0d994510d2664cd4", "expert": true }
],
"cbor": "a1018da20168436861696e20696402686d792d636861696ea2016e4163636f756e74206e756d626572026131a2016853657175656e6365026132a301674164647265737302782d636f736d6f7331756c6176336873656e7570737771666b77327933737570356b677471776e767161386579687304f5a3016a5075626c6963206b657902781f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657904f5a401634b657902785230324542204444374620453446442045423736204443384120323035452046363544203739304320443330452038413337203541354320323532382045423341203932334120463146422034443739203444030104f5a102781e54686973207472616e73616374696f6e206861732031204d657373616765a3016d4d6573736167652028312f312902622f410301a3016542595445530278575348412d3235363d333242412035343543204430373020334530392030464643204438304620323045372031373239203944313220354434362033373238203838373120324232442042324437204346443220414138300302a1026e456e64206f66204d657373616765a2016446656573026a302e3030322041544f4da30169476173206c696d697402673130302730303004f5a3017148617368206f66207261772062797465730278403034323431666266613333366238326237666139643361643564383730363839313739386161396134393738646139653064393934353130643236363463643404f5"
},
{
"name": "minimal with bytes",
"proto": {
"body": {
"messages": [
{
"@type": "/A",
"BYTES": "0x123123"
}
]
},
"auth_info": {
"signer_infos": [
{
"public_key": {
"@type": "/cosmos.crypto.secp256k1.PubKey",
"key": "Auvdf+T963bciiBe9l15DNMOijdaXCUo6zqSOvH7TXlN"
},
"mode_info": { "single": { "mode": "SIGN_MODE_TEXTUAL" } },
"sequence": 2
}
],
"fee": {
"amount": [{ "denom": "uatom", "amount": "2000" }],
"gas_limit": 100000
}
}
},
"signer_data": {
"account_number": 1,
"address": "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs",
"chain_id": "my-chain",
"pub_key": {
"@type": "/cosmos.crypto.secp256k1.PubKey",
"key": "Auvdf+T963bciiBe9l15DNMOijdaXCUo6zqSOvH7TXlN"
},
"sequence": 2
},
"metadata": {
"display": "ATOM",
"base": "uatom",
"denom_units": [
{ "denom": "ATOM", "exponent": 6 },
{ "denom": "uatom", "exponent": 0 }
]
},
"screens": [
{ "title": "Chain id", "content": "my-chain" },
{ "title": "Account number", "content": "1" },
{ "title": "Sequence", "content": "2" },
{ "title": "Address", "content": "cosmos1ulav3hsenupswqfkw2y3sup5kgtqwnvqa8eyhs", "expert": true },
{ "title": "Public key", "content": "/cosmos.crypto.secp256k1.PubKey", "expert": true },
{ "title": "Key", "content": "02EB DD7F E4FD EB76 DC8A 205E F65D 790C D30E 8A37 5A5C 2528 EB3A 923A F1FB 4D79 4D", "indent": 1, "expert": true },
{ "content": "This transaction has 1 Message" },
{ "title": "Message (1/1)", "content": "/A", "indent": 1 },
{ "title": "BYTES", "content": "D31D 76DF 5DB7", "indent": 2 },
{ "content": "End of Message" },
{ "title": "Fees", "content": "0.002 ATOM" },
{ "title": "Gas limit", "content": "100'000", "expert": true },
{ "title": "Hash of raw bytes", "content": "6dc9a7a96c0908380dc067f2066d43844b55f430ace369dc165cfa981061d8cf", "expert": true }
],
"cbor": "a1018da20168436861696e20696402686d792d636861696ea2016e4163636f756e74206e756d626572026131a2016853657175656e6365026132a301674164647265737302782d636f736d6f7331756c6176336873656e7570737771666b77327933737570356b677471776e767161386579687304f5a3016a5075626c6963206b657902781f2f636f736d6f732e63727970746f2e736563703235366b312e5075624b657904f5a401634b657902785230324542204444374620453446442045423736204443384120323035452046363544203739304320443330452038413337203541354320323532382045423341203932334120463146422034443739203444030104f5a102781e54686973207472616e73616374696f6e206861732031204d657373616765a3016d4d6573736167652028312f312902622f410301a301654259544553026e44333144203736444620354442370302a1026e456e64206f66204d657373616765a2016446656573026a302e3030322041544f4da30169476173206c696d697402673130302730303004f5a3017148617368206f66207261772062797465730278403664633961376139366330393038333830646330363766323036366434333834346235356634333061636533363964633136356366613938313036316438636604f5"
},
{
"name": "a bit of everything",
"proto": {

14
x/tx/signing/textual/message.go
View File

@ -13,6 +13,11 @@ import (
"google.golang.org/protobuf/reflect/protoregistry"
)
var (
headerRegex = regexp.MustCompile(`(\d+) .+`)
elementRegex = regexp.MustCompile(`(.+) \(\d+\/\d+\)`)
)
type messageValueRenderer struct {
tr *SignModeHandler
msgDesc protoreflect.MessageDescriptor
@ -181,9 +186,8 @@ func (mr *messageValueRenderer) Parse(ctx context.Context, screens []Screen) (pr
return nilValue, errors.New("expect at least one screen")
}
wantHeader := fmt.Sprintf("%s object", mr.msgDesc.Name())
if screens[0].Content != wantHeader {
return nilValue, fmt.Errorf(`bad header: want "%s", got "%s"`, wantHeader, screens[0].Title)
if screens[0].Content != mr.header() {
return nilValue, fmt.Errorf(`bad header: want "%s", got "%s"`, mr.header(), screens[0].Title)
}
if screens[0].Indent != 0 {
return nilValue, fmt.Errorf("bad message indentation: want 0, got %d", screens[0].Indent)
@ -261,9 +265,6 @@ func (mr *messageValueRenderer) Parse(ctx context.Context, screens []Screen) (pr
return protoreflect.ValueOfMessage(msg), nil
}
// <int> <field_kind>
var headerRegex = regexp.MustCompile(`(\d+) .+`)
func (mr *messageValueRenderer) parseRepeated(ctx context.Context, screens []Screen, l protoreflect.List, vr ValueRenderer) error {
res := headerRegex.FindAllStringSubmatch(screens[0].Content, -1)
if res == nil {
@ -280,7 +281,6 @@ func (mr *messageValueRenderer) parseRepeated(ctx context.Context, screens []Scr
elementIndex := 1
// <field_name> (<int>/<int>): <value rendered 1st line>
elementRegex := regexp.MustCompile(`(.+) \(\d+\/\d+\)`)
elementRes := elementRegex.FindAllStringSubmatch(screens[idx].Title, -1)
if elementRes == nil {
return errors.New("element malformed")

2
x/tx/signing/textual/string.go
View File

@ -21,7 +21,7 @@ func (sr stringValueRenderer) Format(_ context.Context, v protoreflect.Value) ([
func (sr stringValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
if len(screens) != 1 {
return protoreflect.Value{}, fmt.Errorf("expected single screen: %v", screens)
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}
return protoreflect.ValueOfString(screens[0].Content), nil
}

4
x/tx/signing/textual/timestamp.go
View File

@ -36,11 +36,11 @@ func (vr timestampValueRenderer) Format(_ context.Context, v protoreflect.Value)
func (vr timestampValueRenderer) Parse(_ context.Context, screens []Screen) (protoreflect.Value, error) {
// Parse the RFC 3339 input as a Go Time.
if len(screens) != 1 {
return protoreflect.Value{}, fmt.Errorf("expected single screen: %v", screens)
return nilValue, fmt.Errorf("expected single screen: %v", screens)
}
t, err := time.Parse(time.RFC3339Nano, screens[0].Content)
if err != nil {
return protoreflect.Value{}, err
return nilValue, err
}
// Convert Go Time to a proto Timestamp.

3
x/tx/signing/textual/tx.go
View File

@ -138,8 +138,7 @@ func (vr txValueRenderer) Format(ctx context.Context, v protoreflect.Value) ([]S
for i := range screens {
if screens[i].Indent == 0 {
// Do expert fields.
_, ok := expert[screens[i].Title]
if ok {
if _, ok := expert[screens[i].Title]; ok {
expertify(screens, i, screens[i].Title)
}