Skip to content

Commit

Permalink
fix(vm): avoid "index out of range" in convertArgToGno (gnolang#2500)
Browse files Browse the repository at this point in the history 
With this gno code:

```go
// foo.gno
func Foo(val uint64)
```

Currently, passing an empty string where a numeric argument is expected
in a vm call such as:

```go
msg := MsgCall{
  // (...)
  Func: "Foo",
  Args: []string{""},
}
vmkeeper.Call(ctx, msg)
```

Will error out with:
```
runtime error: index out of range [0] with length 0
```

Because the argument conversion routine accesses the first character to
see if it's a `+` without first checking the length of the string

This runtime error is confusing IMO and this PR makes the resulting
error more meaningful, for example:

```
error parsing uint64 "": strconv.ParseUint: parsing "": invalid syntax
```

also increases `gnoland` code coverage by ~2% as a bonus ;)

---------

Signed-off-by: Norman Meier <[email protected]>
  • Loading branch information
@n0izn0iz @gfanton
n0izn0iz authored and gfanton committed Jul 23, 2024
1 parent 3a3acac commit 4fcc875
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 15 deletions.
31 changes: 16 additions & 15 deletions gno.land/pkg/sdk/vm/convert.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,14 @@ import (
"encoding/base64"
"fmt"
"strconv"
"strings"

"github.com/cockroachdb/apd/v3"
gno "github.com/gnolang/gno/gnovm/pkg/gnolang"
)

func assertCharNotPlus(b byte) {
if b == '+' {
func assertNoPlusPrefix(s string) {
if strings.HasPrefix(s, "+") {
panic("numbers cannot start with +")
}
}
Expand Down Expand Up @@ -41,7 +42,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetString(gno.StringValue(arg))
return
case gno.IntType:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
i64, err := strconv.ParseInt(arg, 10, 64)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -51,7 +52,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetInt(int(i64))
return
case gno.Int8Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
i8, err := strconv.ParseInt(arg, 10, 8)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -61,7 +62,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetInt8(int8(i8))
return
case gno.Int16Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
i16, err := strconv.ParseInt(arg, 10, 16)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -71,7 +72,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetInt16(int16(i16))
return
case gno.Int32Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
i32, err := strconv.ParseInt(arg, 10, 32)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -81,7 +82,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetInt32(int32(i32))
return
case gno.Int64Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
i64, err := strconv.ParseInt(arg, 10, 64)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -91,7 +92,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetInt64(i64)
return
case gno.UintType:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
u64, err := strconv.ParseUint(arg, 10, 64)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -101,7 +102,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetUint(uint(u64))
return
case gno.Uint8Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
u8, err := strconv.ParseUint(arg, 10, 8)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -111,7 +112,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetUint8(uint8(u8))
return
case gno.Uint16Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
u16, err := strconv.ParseUint(arg, 10, 16)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -121,7 +122,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetUint16(uint16(u16))
return
case gno.Uint32Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
u32, err := strconv.ParseUint(arg, 10, 32)
if err != nil {
panic(fmt.Sprintf(
Expand All @@ -131,7 +132,7 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
tv.SetUint32(uint32(u32))
return
case gno.Uint64Type:
assertCharNotPlus(arg[0])
assertNoPlusPrefix(arg)
u64, err := strconv.ParseUint(arg, 10, 64)
if err != nil {
panic(fmt.Sprintf(
Expand Down Expand Up @@ -192,15 +193,15 @@ func convertArgToGno(arg string, argT gno.Type) (tv gno.TypedValue) {
}

func convertFloat(value string, precision int) float64 {
assertCharNotPlus(value[0])
assertNoPlusPrefix(value)
dec, _, err := apd.NewFromString(value)
if err != nil {
panic(fmt.Sprintf("error parsing float%d %s: %v", precision, value, err))
panic(fmt.Sprintf("error parsing float%d %q: %v", precision, value, err))
}

f64, err := strconv.ParseFloat(dec.String(), precision)
if err != nil {
panic(fmt.Sprintf("error value exceeds float%d precision %s: %v", precision, value, err))
panic(fmt.Sprintf("error value exceeds float%d precision %q: %v", precision, value, err))
}

return f64
Expand Down
39 changes: 39 additions & 0 deletions gno.land/pkg/sdk/vm/convert_test.go
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
package vm

import (
"fmt"
"testing"

"github.com/gnolang/gno/gnovm/pkg/gnolang"
"github.com/stretchr/testify/assert"
)

func TestConvertEmptyNumbers(t *testing.T) {
tests := []struct {
argT gnolang.Type
expectedErr string
}{
{gnolang.UintType, `error parsing uint "": strconv.ParseUint: parsing "": invalid syntax`},
{gnolang.Uint64Type, `error parsing uint64 "": strconv.ParseUint: parsing "": invalid syntax`},
{gnolang.Uint32Type, `error parsing uint32 "": strconv.ParseUint: parsing "": invalid syntax`},
{gnolang.Uint16Type, `error parsing uint16 "": strconv.ParseUint: parsing "": invalid syntax`},
{gnolang.Uint8Type, `error parsing uint8 "": strconv.ParseUint: parsing "": invalid syntax`},
{gnolang.IntType, `error parsing int "": strconv.ParseInt: parsing "": invalid syntax`},
{gnolang.Int64Type, `error parsing int64 "": strconv.ParseInt: parsing "": invalid syntax`},
{gnolang.Int32Type, `error parsing int32 "": strconv.ParseInt: parsing "": invalid syntax`},
{gnolang.Int16Type, `error parsing int16 "": strconv.ParseInt: parsing "": invalid syntax`},
{gnolang.Int8Type, `error parsing int8 "": strconv.ParseInt: parsing "": invalid syntax`},
{gnolang.Float64Type, `error parsing float64 "": parse mantissa: `},
{gnolang.Float32Type, `error parsing float32 "": parse mantissa: `},
}

for _, tt := range tests {
testname := fmt.Sprintf("%v", tt.argT)
t.Run(testname, func(t *testing.T) {
run := func() {
_ = convertArgToGno("", tt.argT)
}
assert.PanicsWithValue(t, tt.expectedErr, run)
})
}
}

0 comments on commit 4fcc875

Please sign in to comment.