mempool CheckTx doesn't check maxMsgSize but checks msg size from peers

hi, recently I was considering memo size, and I found that mempool CheckTx doesn't check maxMsgSize but check msg size from peers which will cause validator that receive wrong size msg closed by remote validators.

eg, validator0 add a wrong size msg in mempool and it will broadcast it to other validators.
```
I[12-12|08:14:44.953] Added good transaction                       module=mempool tx=E67C405C6CC2F38568BA04C8C98032B2A991FBE4
```

but other validators will receive wrong size msg, and will close the connection to validator0.
```
E[12-12|08:14:44.964] Error decoding message                       module=mempool src="Peer{MConn{172.20.0.2:26656} e78d2515ac66c324a4cdd6feb6c4fe181b4ea5d6 out}" chId=48 msg=null err="Msg exceeds max size (209 > 100)"
E[12-12|08:14:44.964] Stopping peer for error                      module=p2p peer="Peer{MConn{172.20.0.2:26656} e78d2515ac66c324a4cdd6feb6c4fe181b4ea5d6 out}" err="Msg exceeds max size (209 > 100)"
```

the reason is we do not check tx size here: https://github.com/tendermint/tendermint/blob/8003786c9affff242861141bf7484aeb5796e42c/mempool/mempool.go#L303-L348

but we check tx size here: https://github.com/tendermint/tendermint/blob/8003786c9affff242861141bf7484aeb5796e42c/mempool/reactor.go#L184-L190

if we don't check tx size before we send to peers, especially in validators, we can easily be hacked by malicious users.

	func (mem Mempool) CheckTx(tx types.Tx, cb func(abci.Response)) (err error) {
	mem.proxyMtx.Lock()
	// use defer to unlock mutex because application (local client) might panic
	defer mem.proxyMtx.Unlock()

	if mem.Size() >= mem.config.Size {
	return ErrMempoolIsFull
	}

	if mem.preCheck != nil {
	if err := mem.preCheck(tx); err != nil {
	return ErrPreCheck{err}
	}
	}

	// CACHE
	if !mem.cache.Push(tx) {
	return ErrTxInCache
	}
	// END CACHE

	// WAL
	if mem.wal != nil {
	// TODO: Notify administrators when WAL fails
	_, err := mem.wal.Write([]byte(tx))
	if err != nil {
	mem.logger.Error("Error writing to WAL", "err", err)
	}
	_, err = mem.wal.Write([]byte("\n"))
	if err != nil {
	mem.logger.Error("Error writing to WAL", "err", err)
	}
	}
	// END WAL

	// NOTE: proxyAppConn may error if tx buffer is full
	if err = mem.proxyAppConn.Error(); err != nil {
	return err
	}
	reqRes := mem.proxyAppConn.CheckTxAsync(tx)
	if cb != nil {
	reqRes.SetCallback(cb)
	}

	return nil
	}

	func decodeMsg(bz []byte) (msg MempoolMessage, err error) {
	if len(bz) > maxMsgSize {
	return msg, fmt.Errorf("Msg exceeds max size (%d > %d)", len(bz), maxMsgSize)
	}
	err = cdc.UnmarshalBinaryBare(bz, &msg)
	return
	}

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

mempool CheckTx doesn't check maxMsgSize but checks msg size from peers #3008

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

mempool CheckTx doesn't check maxMsgSize but checks msg size from peers #3008

Description

Metadata

Metadata

Assignees

Labels

Type

Projects

Milestone

Relationships

Development

Issue actions