From e1be34bce1ddf662bca58a37a6f38fea63a2a70f Mon Sep 17 00:00:00 2001 From: zelig Date: Sat, 28 Mar 2015 00:48:37 +0000 Subject: eth: SEC-29 eth wire protocol decoding invalid message data crashes client - add validate method to types.Block - validate after Decode -> error - add tests for NewBlockMsg --- eth/protocol.go | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) (limited to 'eth/protocol.go') diff --git a/eth/protocol.go b/eth/protocol.go index e32ea233b..0a3f67b62 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -105,7 +105,7 @@ type getBlockHashesMsgData struct { type statusMsgData struct { ProtocolVersion uint32 NetworkId uint32 - TD *big.Int + TD big.Int CurrentBlock common.Hash GenesisBlock common.Hash } @@ -276,6 +276,9 @@ func (self *ethProtocol) handle() error { if err := msg.Decode(&request); err != nil { return self.protoError(ErrDecode, "%v: %v", msg, err) } + if err := request.Block.Validate(); err != nil { + return self.protoError(ErrDecode, "block validation %v: %v", msg, err) + } hash := request.Block.Hash() _, chainHead, _ := self.chainManager.Status() @@ -335,7 +338,7 @@ func (self *ethProtocol) handleStatus() error { return self.protoError(ErrProtocolVersionMismatch, "%d (!= %d)", status.ProtocolVersion, self.protocolVersion) } - _, suspended := self.blockPool.AddPeer(status.TD, status.CurrentBlock, self.id, self.requestBlockHashes, self.requestBlocks, self.protoErrorDisconnect) + _, suspended := self.blockPool.AddPeer(&status.TD, status.CurrentBlock, self.id, self.requestBlockHashes, self.requestBlocks, self.protoErrorDisconnect) if suspended { return self.protoError(ErrSuspendedPeer, "") } @@ -366,7 +369,7 @@ func (self *ethProtocol) sendStatus() error { return p2p.Send(self.rw, StatusMsg, &statusMsgData{ ProtocolVersion: uint32(self.protocolVersion), NetworkId: uint32(self.networkId), - TD: td, + TD: *td, CurrentBlock: currentBlock, GenesisBlock: genesisBlock, }) -- cgit v1.2.3 From 82da6bf4d213784cfc7ba45432f9f96c2d6b4d9d Mon Sep 17 00:00:00 2001 From: zelig Date: Mon, 30 Mar 2015 13:48:07 +0100 Subject: test for invalid rlp encoding of block in BlocksMsg - rename Validate -> ValidateFields not to confure consensus block validation - add nil transaction and nil uncle header validation - remove bigint field checks: rlp already decodes *big.Int to big.NewInt(0) - add test for nil header, nil transaction --- eth/protocol.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'eth/protocol.go') diff --git a/eth/protocol.go b/eth/protocol.go index 0a3f67b62..f0a749d33 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -268,6 +268,9 @@ func (self *ethProtocol) handle() error { return self.protoError(ErrDecode, "msg %v: %v", msg, err) } } + if err := block.ValidateFields(); err != nil { + return self.protoError(ErrDecode, "block validation %v: %v", msg, err) + } self.blockPool.AddBlock(&block, self.id) } @@ -276,7 +279,7 @@ func (self *ethProtocol) handle() error { if err := msg.Decode(&request); err != nil { return self.protoError(ErrDecode, "%v: %v", msg, err) } - if err := request.Block.Validate(); err != nil { + if err := request.Block.ValidateFields(); err != nil { return self.protoError(ErrDecode, "block validation %v: %v", msg, err) } hash := request.Block.Hash() -- cgit v1.2.3 From 6ffea34d8bf19fb358c1a7f01e20bf25f1061c5e Mon Sep 17 00:00:00 2001 From: zelig Date: Mon, 30 Mar 2015 15:21:41 +0100 Subject: check TxMsg - add validation on TxMsg checking for nil - add test for nil transaction - add test for zero value transaction (no extra validation needed) --- eth/protocol.go | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) (limited to 'eth/protocol.go') diff --git a/eth/protocol.go b/eth/protocol.go index f0a749d33..214eed875 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -185,7 +185,10 @@ func (self *ethProtocol) handle() error { if err := msg.Decode(&txs); err != nil { return self.protoError(ErrDecode, "msg %v: %v", msg, err) } - for _, tx := range txs { + for i, tx := range txs { + if tx == nil { + return self.protoError(ErrDecode, "transaction %d is nil", i) + } jsonlogger.LogJson(&logger.EthTxReceived{ TxHash: tx.Hash().Hex(), RemoteId: self.peer.ID().String(), -- cgit v1.2.3 From f56fc9cd9d13e88ee1a244ea590e249e324b8b84 Mon Sep 17 00:00:00 2001 From: zelig Date: Wed, 1 Apr 2015 12:36:49 +0100 Subject: change StatusMsgData.TD back to pointer type *big.Int --- eth/protocol.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) (limited to 'eth/protocol.go') diff --git a/eth/protocol.go b/eth/protocol.go index 214eed875..a0ab177cd 100644 --- a/eth/protocol.go +++ b/eth/protocol.go @@ -105,7 +105,7 @@ type getBlockHashesMsgData struct { type statusMsgData struct { ProtocolVersion uint32 NetworkId uint32 - TD big.Int + TD *big.Int CurrentBlock common.Hash GenesisBlock common.Hash } @@ -344,7 +344,7 @@ func (self *ethProtocol) handleStatus() error { return self.protoError(ErrProtocolVersionMismatch, "%d (!= %d)", status.ProtocolVersion, self.protocolVersion) } - _, suspended := self.blockPool.AddPeer(&status.TD, status.CurrentBlock, self.id, self.requestBlockHashes, self.requestBlocks, self.protoErrorDisconnect) + _, suspended := self.blockPool.AddPeer(status.TD, status.CurrentBlock, self.id, self.requestBlockHashes, self.requestBlocks, self.protoErrorDisconnect) if suspended { return self.protoError(ErrSuspendedPeer, "") } @@ -375,7 +375,7 @@ func (self *ethProtocol) sendStatus() error { return p2p.Send(self.rw, StatusMsg, &statusMsgData{ ProtocolVersion: uint32(self.protocolVersion), NetworkId: uint32(self.networkId), - TD: *td, + TD: td, CurrentBlock: currentBlock, GenesisBlock: genesisBlock, }) -- cgit v1.2.3