gnolang / gno

Gno: An interpreted, stack-based Go virtual machine to build succinct and composable apps + gno.land: a blockchain for timeless code and fair open-source.
https://gno.land/
Other
898 stars 374 forks source link

gnovm: avoid letting malformed strings like ".-400" be passed into apd.Decimal or send patch to cockroachdb/apd #2968

Open odeke-em opened 1 month ago

odeke-em commented 1 month ago

Description

I found this bug by fuzzing with this fuzzer

package gnolang

import (
    "os"
    "path/filepath"
    "strings"
    "testing"

    "github.com/cockroachdb/apd/v3"
)

func FuzzConvertUntypedBigdecToFloat(f *testing.F) {
    // 1. Firstly add seeds.
    seeds := []string{
        "-100000",
        "100000",
        "0",
    }

    check := new(apd.Decimal)
    for _, seed := range seeds {
        if check.UnmarshalText([]byte(seed)) == nil {
            f.Add(seed)
        }
    }

    f.Fuzz(func(t *testing.T, apdStr string) {
        switch {
        case strings.HasPrefix(apdStr, ".-"):
            return
        }

        v := new(apd.Decimal)
        if err := v.UnmarshalText([]byte(apdStr)); err != nil {
            return
        }
        if _, err := v.Float64(); err != nil {
            return
        }

        bd := BigdecValue{
            V: v,
        }
        dst := new(TypedValue)
        typ := Float64Type
        ConvertUntypedBigdecTo(dst, bd, typ)
    })
}

we re-discovered a long standing bug in cockroachdb/apd https://github.com/cockroachdb/apd/issues/120#issuecomment-2417135080 let's be cautious that strings of that form will cause a panic in gnovm hence either send a fix to apd and can even use it to market gnolang and contribute positively or at least protect against such strings

kristovatlas commented 3 weeks ago

Thanks for the report, @odeke-em. We're looking into it. And extra thanks for providing the fuzzer source.