Merge pull request #3538 from filecoin-project/steb/safe-exports

Robustify state manager against holes in actor method numbers

chain/stmgr/utils.go

@@ -3,8 +3,11 @@ package stmgr
 import (
 	"bytes"
 	"context"
+	"fmt"
 	"os"
 	"reflect"
+	"runtime"
+	"strings"
 
 	cid "github.com/ipfs/go-cid"
 	cbor "github.com/ipfs/go-ipld-cbor"
@@ -586,14 +589,14 @@ func MinerGetBaseInfo(ctx context.Context, sm *StateManager, bcn beacon.RandomBe
 	}, nil
 }
 
-type methodMeta struct {
+type MethodMeta struct {
 	Name string
 
 	Params reflect.Type
 	Ret    reflect.Type
 }
 
-var MethodsMap = map[cid.Cid][]methodMeta{}
+var MethodsMap = map[cid.Cid]map[abi.MethodNum]MethodMeta{}
 
 func init() {
 	cidToMethods := map[cid.Cid][2]interface{}{
@@ -611,25 +614,65 @@ func init() {
 	}
 
 	for c, m := range cidToMethods {
-		rt := reflect.TypeOf(m[0])
-		nf := rt.NumField()
+		exports := m[1].(abi.Invokee).Exports()
+		methods := make(map[abi.MethodNum]MethodMeta, len(exports))
 
-		MethodsMap[c] = append(MethodsMap[c], methodMeta{
+		// Explicitly add send, it's special.
+		methods[builtin.MethodSend] = MethodMeta{
 			Name:   "Send",
 			Params: reflect.TypeOf(new(adt.EmptyValue)),
 			Ret:    reflect.TypeOf(new(adt.EmptyValue)),
-		})
-
-		exports := m[1].(abi.Invokee).Exports()
-		for i := 0; i < nf; i++ {
-			export := reflect.TypeOf(exports[i+1])
-
-			MethodsMap[c] = append(MethodsMap[c], methodMeta{
-				Name:   rt.Field(i).Name,
-				Params: export.In(1),
-				Ret:    export.Out(0),
-			})
 		}
+
+		// Learn method names from the builtin.Methods* structs.
+		rv := reflect.ValueOf(m[0])
+		rt := rv.Type()
+		nf := rt.NumField()
+		methodToName := make([]string, len(exports))
+		for i := 0; i < nf; i++ {
+			name := rt.Field(i).Name
+			number := rv.Field(i).Interface().(abi.MethodNum)
+			methodToName[number] = name
+		}
+
+		// Iterate over exported methods. Some of these _may_ be nil and
+		// must be skipped.
+		for number, export := range exports {
+			if export == nil {
+				continue
+			}
+
+			ev := reflect.ValueOf(export)
+			et := ev.Type()
+
+			// Make sure the method name is correct.
+			// This is just a nice sanity check.
+			fnName := runtime.FuncForPC(ev.Pointer()).Name()
+			fnName = strings.TrimSuffix(fnName[strings.LastIndexByte(fnName, '.')+1:], "-fm")
+			mName := methodToName[number]
+			if mName != fnName {
+				panic(fmt.Sprintf(
+					"actor method name is %s but exported method name is %s",
+					fnName, mName,
+				))
+			}
+
+			switch abi.MethodNum(number) {
+			case builtin.MethodSend:
+				panic("method 0 is reserved for Send")
+			case builtin.MethodConstructor:
+				if fnName != "Constructor" {
+					panic("method 1 is reserved for Constructor")
+				}
+			}
+
+			methods[abi.MethodNum(number)] = MethodMeta{
+				Name:   fnName,
+				Params: et.In(1),
+				Ret:    et.Out(0),
+			}
+		}
+		MethodsMap[c] = methods
 	}
 }
 
@@ -639,7 +682,10 @@ func GetReturnType(ctx context.Context, sm *StateManager, to address.Address, me
 		return nil, xerrors.Errorf("getting actor: %w", err)
 	}
 
-	m := MethodsMap[act.Code][method]
+	m, found := MethodsMap[act.Code][method]
+	if !found {
+		return nil, fmt.Errorf("unknown method %d for actor %s", method, act.Code)
+	}
 	return reflect.New(m.Ret.Elem()).Interface().(cbg.CBORUnmarshaler), nil
 }
 

chain/vm/invoker.go

@@ -130,6 +130,9 @@ func (*Invoker) transform(instance Invokee) (nativeCode, error) {
 	}
 	code := make(nativeCode, len(exports))
 	for id, m := range exports {
+		if m == nil {
+			continue
+		}
 		meth := reflect.ValueOf(m)
 		code[id] = reflect.MakeFunc(reflect.TypeOf((invokeFunc)(nil)),
 			func(in []reflect.Value) []reflect.Value {

cli/send.go

@@ -173,7 +173,12 @@ func decodeTypedParams(ctx context.Context, fapi api.FullNode, to address.Addres
 		return nil, err
 	}
 
-	p := reflect.New(stmgr.MethodsMap[act.Code][method].Params.Elem()).Interface().(cbg.CBORMarshaler)
+	methodMeta, found := stmgr.MethodsMap[act.Code][method]
+	if !found {
+		return nil, fmt.Errorf("method %d not found on actor %s", method, act.Code)
+	}
+
+	p := reflect.New(methodMeta.Params.Elem()).Interface().(cbg.CBORMarshaler)
 
 	if err := json.Unmarshal([]byte(paramstr), p); err != nil {
 		return nil, fmt.Errorf("unmarshaling input into params type: %w", err)

cli/state.go

@@ -1167,7 +1167,11 @@ func sumGas(changes []*types.GasTrace) types.GasTrace {
 }
 
 func jsonParams(code cid.Cid, method abi.MethodNum, params []byte) (string, error) {
-	re := reflect.New(stmgr.MethodsMap[code][method].Params.Elem())
+	methodMeta, found := stmgr.MethodsMap[code][method]
+	if !found {
+		return "", fmt.Errorf("method %d not found on actor %s", method, code)
+	}
+	re := reflect.New(methodMeta.Params.Elem())
 	p := re.Interface().(cbg.CBORUnmarshaler)
 	if err := p.UnmarshalCBOR(bytes.NewReader(params)); err != nil {
 		return "", err
@@ -1178,7 +1182,11 @@ func jsonParams(code cid.Cid, method abi.MethodNum, params []byte) (string, erro
 }
 
 func jsonReturn(code cid.Cid, method abi.MethodNum, ret []byte) (string, error) {
-	re := reflect.New(stmgr.MethodsMap[code][method].Ret.Elem())
+	methodMeta, found := stmgr.MethodsMap[code][method]
+	if !found {
+		return "", fmt.Errorf("method %d not found on actor %s", method, code)
+	}
+	re := reflect.New(methodMeta.Ret.Elem())
 	p := re.Interface().(cbg.CBORUnmarshaler)
 	if err := p.UnmarshalCBOR(bytes.NewReader(ret)); err != nil {
 		return "", err