cerc-io/lotus
15
0
Fork 0
Code Issues Pull Requests Packages Projects Releases 27 Wiki Activity

feat: api: sanity check the "to" address of outgoing messages (#12135)

Browse Source 
* feat: api: sanity check the "to" address of outgoing messages

If the "to" address of an outgoing message is a _delegated_ address,
verify that it maps to a valid Ethereum address. This isn't a consensus
critical change, but it'll help prevent client-side address conversion
libraries from directing messages into oblivion (e.g., by
mis-translating `0xff0000....` addresses into `f410f...` addresses
instead of `f0...` addresses.

* tests for invalid delegated addresses

* fix lint

---------

Co-authored-by: aarshkshah1992 <aarshkshah1992@gmail.com>
This commit is contained in:
Steven Allen 2024-07-03 01:58:34 +00:00 committed by Aarsh Shah
parent 85abc61c17
commit 58c029a63b
3 changed files with 137 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
itests/mempool_test.go
View File

@ -8,6 +8,7 @@ import (
"github.com/stretchr/testify/require"
"github.com/filecoin-project/go-address"
"github.com/filecoin-project/go-state-types/big"
"github.com/filecoin-project/lotus/api"
@ -18,6 +19,33 @@ import (
const mPoolThrottle = time.Millisecond * 100
const mPoolTimeout = time.Second * 10
func TestMemPoolPushOutgoingInvalidDelegated(t *testing.T) {
//stm: @CHAIN_MEMPOOL_PENDING_001, @CHAIN_STATE_WAIT_MSG_001, @CHAIN_MEMPOOL_CAP_GAS_FEE_001
//stm: @CHAIN_MEMPOOL_PUSH_002
ctx := context.Background()
firstNode, _, _, ens := kit.EnsembleTwoOne(t, kit.MockProofs())
ens.InterconnectAll()
kit.QuietMiningLogs()
sender := firstNode.DefaultKey.Address
badTo, err := address.NewFromString("f410f74aaaaaaaaaaaaaaaaaaaaaaaaac5sh2bf3lgta")
require.NoError(t, err)
bal, err := firstNode.WalletBalance(ctx, sender)
require.NoError(t, err)
toSend := big.Div(bal, big.NewInt(10))
msg := &types.Message{
From: sender,
Value: toSend,
To: badTo,
}
_, err = firstNode.MpoolPushMessage(ctx, msg, nil)
require.Error(t, err)
require.Contains(t, err.Error(), "is a delegated address but not a valid Eth Address")
}
func TestMemPoolPushSingleNode(t *testing.T) {
//stm: @CHAIN_MEMPOOL_CREATE_MSG_CHAINS_001, @CHAIN_MEMPOOL_SELECT_001
//stm: @CHAIN_MEMPOOL_PENDING_001, @CHAIN_STATE_WAIT_MSG_001, @CHAIN_MEMPOOL_CAP_GAS_FEE_001

42
node/impl/full/mpool.go
View File

@ -16,6 +16,7 @@ import (
"github.com/filecoin-project/lotus/chain/messagepool"
"github.com/filecoin-project/lotus/chain/messagesigner"
"github.com/filecoin-project/lotus/chain/types"
"github.com/filecoin-project/lotus/chain/types/ethtypes"
"github.com/filecoin-project/lotus/node/modules/dtypes"
)
@ -131,14 +132,24 @@ func (a *MpoolAPI) MpoolClear(ctx context.Context, local bool) error {
}
func (m *MpoolModule) MpoolPush(ctx context.Context, smsg *types.SignedMessage) (cid.Cid, error) {
if err := sanityCheckOutgoingMessage(&smsg.Message); err != nil {
return cid.Undef, xerrors.Errorf("message %s from %s with nonce %d failed sanity check: %w", smsg.Cid(), smsg.Message.From, smsg.Message.Nonce, err)
}
return m.Mpool.Push(ctx, smsg, true)
}
func (a *MpoolAPI) MpoolPushUntrusted(ctx context.Context, smsg *types.SignedMessage) (cid.Cid, error) {
if err := sanityCheckOutgoingMessage(&smsg.Message); err != nil {
return cid.Undef, xerrors.Errorf("message %s from %s with nonce %d failed sanity check: %w", smsg.Cid(), smsg.Message.From, smsg.Message.Nonce, err)
}
return a.Mpool.PushUntrusted(ctx, smsg)
}
func (a *MpoolAPI) MpoolPushMessage(ctx context.Context, msg *types.Message, spec *api.MessageSendSpec) (*types.SignedMessage, error) {
if err := sanityCheckOutgoingMessage(msg); err != nil {
return nil, xerrors.Errorf("message from %s failed sanity check: %w", msg.From, err)
}
cp := *msg
msg = &cp
inMsg := *msg
@ -223,6 +234,11 @@ func (a *MpoolAPI) MpoolPushMessage(ctx context.Context, msg *types.Message, spe
}
func (a *MpoolAPI) MpoolBatchPush(ctx context.Context, smsgs []*types.SignedMessage) ([]cid.Cid, error) {
for _, msg := range smsgs {
if err := sanityCheckOutgoingMessage(&msg.Message); err != nil {
return nil, xerrors.Errorf("message %s from %s with nonce %d failed sanity check: %w", msg.Cid(), msg.Message.From, msg.Message.Nonce, err)
}
}
var messageCids []cid.Cid
for _, smsg := range smsgs {
smsgCid, err := a.Mpool.Push(ctx, smsg, true)
@ -235,6 +251,11 @@ func (a *MpoolAPI) MpoolBatchPush(ctx context.Context, smsgs []*types.SignedMess
}
func (a *MpoolAPI) MpoolBatchPushUntrusted(ctx context.Context, smsgs []*types.SignedMessage) ([]cid.Cid, error) {
for _, msg := range smsgs {
if err := sanityCheckOutgoingMessage(&msg.Message); err != nil {
return nil, xerrors.Errorf("message %s from %s with nonce %d failed sanity check: %w", msg.Cid(), msg.Message.From, msg.Message.Nonce, err)
}
}
var messageCids []cid.Cid
for _, smsg := range smsgs {
smsgCid, err := a.Mpool.PushUntrusted(ctx, smsg)
@ -247,6 +268,11 @@ func (a *MpoolAPI) MpoolBatchPushUntrusted(ctx context.Context, smsgs []*types.S
}
func (a *MpoolAPI) MpoolBatchPushMessage(ctx context.Context, msgs []*types.Message, spec *api.MessageSendSpec) ([]*types.SignedMessage, error) {
for i, msg := range msgs {
if err := sanityCheckOutgoingMessage(msg); err != nil {
return nil, xerrors.Errorf("message #%d from %s with failed sanity check: %w", i, msg.From, err)
}
}
var smsgs []*types.SignedMessage
for _, msg := range msgs {
smsg, err := a.MpoolPushMessage(ctx, msg, spec)
@ -277,3 +303,19 @@ func (a *MpoolAPI) MpoolGetNonce(ctx context.Context, addr address.Address) (uin
func (a *MpoolAPI) MpoolSub(ctx context.Context) (<-chan api.MpoolUpdate, error) {
return a.Mpool.Updates(ctx)
}
func sanityCheckOutgoingMessage(msg *types.Message) error {
// Check that the message's TO address is a _valid_ Eth address if it's a delegated address.
//
// It's legal (from a consensus perspective) to send funds to any 0xf410f address as long as
// the payload is at most 54 bytes, but the vast majority of this address space is
// essentially a black-hole. Unfortunately, the conversion from 0x addresses to Filecoin
// native addresses has a few pitfalls (especially with respect to masked ID addresses), so
// we've added this check to the API to avoid accidentally (and avoidably) sending messages
// to these black-hole addresses.
if msg.To.Protocol() == address.Delegated && !ethtypes.IsEthAddress(msg.To) {
return xerrors.Errorf("message recipient %s is a delegated address but not a valid Eth Address", msg.To)
}
return nil
}

67
node/impl/full/mpool_test.go Normal file
View File

@ -0,0 +1,67 @@
package full
import (
"context"
"testing"
"github.com/stretchr/testify/require"
"github.com/filecoin-project/go-address"
"github.com/filecoin-project/go-state-types/crypto"
"github.com/filecoin-project/lotus/chain/types"
)
func TestSanityCheckOutgoingMessage(t *testing.T) {
// fails for invalid delegated address
badTo, err := address.NewFromString("f410f74aaaaaaaaaaaaaaaaaaaaaaaaac5sh2bf3lgta")
require.NoError(t, err)
msg := &types.Message{
To: badTo,
}
err = sanityCheckOutgoingMessage(msg)
require.Error(t, err)
require.Contains(t, err.Error(), "is a delegated address but not a valid Eth Address")
// works for valid delegated address
goodTo, err := address.NewFromString("f410faxfebiima2gp4lduo2k3vt2iuqapuk3logeftky")
require.NoError(t, err)
msg = &types.Message{
To: goodTo,
}
err = sanityCheckOutgoingMessage(msg)
require.NoError(t, err)
// works for valid non-delegated address
goodTo, err = address.NewFromString("f1z762skeib2v6zlkvhywmjxbv3dxoiv4hmb6gs4y")
require.NoError(t, err)
msg = &types.Message{
To: goodTo,
}
err = sanityCheckOutgoingMessage(msg)
require.NoError(t, err)
}
func TestMpoolPushInvalidDelegatedAddressFails(t *testing.T) {
badTo, err := address.NewFromString("f410f74aaaaaaaaaaaaaaaaaaaaaaaaac5sh2bf3lgta")
require.NoError(t, err)
module := &MpoolModule{}
m := &MpoolAPI{
MpoolModuleAPI: module,
}
smsg := &types.SignedMessage{
Message: types.Message{
From: badTo,
To: badTo,
},
Signature: crypto.Signature{
Type: crypto.SigTypeSecp256k1,
Data: []byte("signature"),
},
}
_, err = m.MpoolPush(context.Background(), smsg)
require.Error(t, err)
require.Contains(t, err.Error(), "is a delegated address but not a valid Eth Address")
}