Merge pull request #361 from filecoin-project/fix/bad-msg-inclusion

reject messages with bad 'To' addresses

chain/messagepool.go

@@ -21,6 +21,8 @@ var (
 	ErrNonceTooLow = fmt.Errorf("message nonce too low")
 
 	ErrNotEnoughFunds = fmt.Errorf("not enough funds to execute transaction")
+
+	ErrInvalidToAddr = fmt.Errorf("message had invalid to address")
 )
 
 type MessagePool struct {
@@ -98,6 +100,10 @@ func (mp *MessagePool) Add(m *types.SignedMessage) error {
 		return ErrMessageTooBig
 	}
 
+	if m.Message.To == address.Undef {
+		return ErrInvalidToAddr
+	}
+
 	if !m.Message.Value.LessThan(types.TotalFilecoinInt) {
 		return ErrMessageValueTooHigh
 	}

chain/sync.go

@@ -493,9 +493,22 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err
 		return xerrors.Errorf("miner created a block but was not a winner")
 	}
 
+	if err := syncer.checkBlockMessages(ctx, b, baseTs); err != nil {
+		return xerrors.Errorf("block had invalid messages: %w", err)
+	}
+
+	return nil
+}
+
+func (syncer *Syncer) checkBlockMessages(ctx context.Context, b *types.FullBlock, baseTs *types.TipSet) error {
 	nonces := make(map[address.Address]uint64)
 	balances := make(map[address.Address]types.BigInt)
 
+	stateroot, _, err := syncer.sm.TipSetState(ctx, baseTs)
+	if err != nil {
+		return err
+	}
+
 	cst := hamt.CSTFromBstore(syncer.store.Blockstore())
 	st, err := state.LoadStateTree(cst, stateroot)
 	if err != nil {
@@ -503,6 +516,10 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err
 	}
 
 	checkMsg := func(m *types.Message) error {
+		if m.To == address.Undef {
+			return xerrors.New("'To' address cannot be empty")
+		}
+
 		if _, ok := nonces[m.From]; !ok {
 			act, err := st.GetActor(m.From)
 			if err != nil {
@@ -547,7 +564,7 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err
 		pubks = append(pubks, pubk)
 	}
 
-	if err := syncer.verifyBlsAggregate(h.BLSAggregate, sigCids, pubks); err != nil {
+	if err := syncer.verifyBlsAggregate(b.Header.BLSAggregate, sigCids, pubks); err != nil {
 		return xerrors.Errorf("bls aggregate signature was invalid: %w", err)
 	}
 
@@ -588,7 +605,7 @@ func (syncer *Syncer) ValidateBlock(ctx context.Context, b *types.FullBlock) err
 		return err
 	}
 
-	if h.Messages != mrcid {
+	if b.Header.Messages != mrcid {
 		return fmt.Errorf("messages didnt match message root in header")
 	}
 

miner/miner.go

@@ -323,6 +323,11 @@ func selectMessages(ctx context.Context, al actorLookup, base *MiningBase, msgs
 	inclNonces := make(map[address.Address]uint64)
 	inclBalances := make(map[address.Address]types.BigInt)
 	for _, msg := range msgs {
+		if msg.Message.To == address.Undef {
+			log.Warnf("message in mempool had bad 'To' address")
+			continue
+		}
+
 		from := msg.Message.From
 		act, err := al(ctx, from, base.ts)
 		if err != nil {

miner/miner_test.go

@@ -40,6 +40,7 @@ func TestMessageFiltering(t *testing.T) {
 	msgs := []types.Message{
 		types.Message{
 			From:     a1,
+			To:       a1,
 			Nonce:    3,
 			Value:    types.NewInt(500),
 			GasLimit: types.NewInt(50),
@@ -47,6 +48,7 @@ func TestMessageFiltering(t *testing.T) {
 		},
 		types.Message{
 			From:     a1,
+			To:       a1,
 			Nonce:    4,
 			Value:    types.NewInt(500),
 			GasLimit: types.NewInt(50),
@@ -54,6 +56,7 @@ func TestMessageFiltering(t *testing.T) {
 		},
 		types.Message{
 			From:     a2,
+			To:       a1,
 			Nonce:    1,
 			Value:    types.NewInt(800),
 			GasLimit: types.NewInt(100),
@@ -61,6 +64,7 @@ func TestMessageFiltering(t *testing.T) {
 		},
 		types.Message{
 			From:     a2,
+			To:       a1,
 			Nonce:    0,
 			Value:    types.NewInt(800),
 			GasLimit: types.NewInt(100),
@@ -68,6 +72,7 @@ func TestMessageFiltering(t *testing.T) {
 		},
 		types.Message{
 			From:     a2,
+			To:       a1,
 			Nonce:    2,
 			Value:    types.NewInt(150),
 			GasLimit: types.NewInt(100),