Skip to content

Latest commit

 

History

History
211 lines (161 loc) · 8.34 KB

032.md

File metadata and controls

211 lines (161 loc) · 8.34 KB

Sneaky Mercurial Python

High

The implementation of findLastBlockHash() can cause proposal preparation and processing to fail

Summary

The findLastBlockHash() function is implemented wrongly and can cause PrepareProposal() and ProcessProposal() functions to fail returning early with am error message.

As shown below, PrepareProposal() ensures there are no duplicate validators with baseapp.ValidateVoteExtensions() on L93

File: .../babylon/x/checkpointing/proposal.go
068: func (h *ProposalHandler) PrepareProposal() sdk.PrepareProposalHandler {
069: 	return func(ctx sdk.Context, req *abci.RequestPrepareProposal) (*abci.ResponsePrepareProposal, error) {
070: 		// call default handler first to do basic validation
071: 		res, err := h.defaultPrepareProposalHandler(ctx, req)
072: 		if err != nil {
073: 			return nil, fmt.Errorf("failed in default PrepareProposal handler: %w", err)

//////SNIP
092: 		// 1. verify the validity of vote extensions (2/3 majority is achieved)
093: 	@>	err = baseapp.ValidateVoteExtensions(ctx, h.ckptKeeper, req.Height, ctx.ChainID(), req.LocalLastCommit)
094: 		if err != nil {
095: 			return proposalRes, fmt.Errorf("invalid vote extensions: %w", err)
096: 		}
097: 
098: 		// 2. build a checkpoint for the previous epoch
099: 		// Note: the epoch has not increased yet, so
100: 		// we can use the current epoch
101: 	@>	ckpt, err := h.buildCheckpointFromVoteExtensions(ctx, epoch.EpochNumber, req.LocalLastCommit.Votes)

However, when buildCheckpointFromVoteExtensions() is called,

  • Unbonded (removed) validators past extensions are used for evaluation and this can cause the PrepareProposal() to return early. (the same path exists in the ProcessProposal() function
File: .../babylon/x/checkpointing/proposal.go
123: func (h *ProposalHandler) buildCheckpointFromVoteExtensions(ctx sdk.Context, epoch uint64, extendedVotes []abci.ExtendedVoteInfo) (*ckpttypes.RawCheckpointWithMeta, error) {
124: @>	prevBlockID, err := h.findLastBlockHash(extendedVotes)

/////SNIP
130: @>	vals := h.ckptKeeper.GetValidatorSet(ctx, epoch)
131: @>	totalPower := h.ckptKeeper.GetTotalVotingPower(ctx, epoch)




213: func (h *ProposalHandler) findLastBlockHash(extendedVotes []abci.ExtendedVoteInfo) ([]byte, error) {
214: 	// Mapping between block hashes and voting power committed to them
215: 	blockHashes := make(map[string]int64, 0)
216: 	// Iterate over vote extensions and if they have a valid structure
217: 	// increase the voting power of the block hash they commit to
218: 	var totalPower int64 = 0
219: 	for _, vote := range extendedVotes {
220: 		// accumulate voting power from all the votes
221: 		totalPower += vote.Validator.Power
222: 		var ve ckpttypes.VoteExtension
223: 		if len(vote.VoteExtension) == 0 {
224: 			continue
225: 		}
226: 		if err := ve.Unmarshal(vote.VoteExtension); err != nil {
227: 			continue
228: 		}
229: 		if ve.BlockHash == nil {
230: 			continue
231: 		}
232: 		bHash, err := ve.BlockHash.Marshal()
233: 		if err != nil {
234: 			continue
235: 		}
236: 		// Encode the block hash using hex
237: 		blockHashes[hex.EncodeToString(bHash)] += vote.Validator.Power
238: 	}
239: 	var (
240: 		maxPower     int64 = 0
241: 		resBlockHash string
242: 	)
243: 	// Find the block hash that has the maximum voting power committed to it
244: 	for blockHash, power := range blockHashes {
245: 		if power > maxPower {
246: 			resBlockHash = blockHash // block hash that has the maximum voting power committed to it
247: 			maxPower = power
248: 		}
249: 	}
250: 	if len(resBlockHash) == 0 {
251: 		return nil, fmt.Errorf("could not find the block hash")
252: 	}
253: 	// Verify that the voting power committed to the found block hash is
254: 	// more than 2/3 of the total voting power.
255: 	if requiredVP := ((totalPower * 2) / 3) + 1; maxPower < requiredVP {
256: 		return nil, fmt.Errorf(
257: 			"insufficient cumulative voting power received to verify vote extensions; got: %d, expected: >=%d",
258: 			maxPower, requiredVP,
259: 		)
260: 	}
261: 	decoded, err := hex.DecodeString(resBlockHash)
262: 	if err != nil {
263: 		return nil, err
264: 	}
265: 	return decoded, nil
266: }

On L130 - L131 above the validator set of the current epoch are now being used (although there is no guarantee that this does not contain unbounded validators) but the findLastBlockHash() uses extension from previous blocks that may have been signed by validators that have become unbonded.

Root Cause

The problem is that

  • findLastBlockHash() can be evaluated using VoteExtension of currently inactive (unbonded or removed) validators since the last or even prior blocks
  • the findLastBlockHash() implementation also assumes there will be one block in which the cummulative voting power of extensions in that block exceeds 2/3 of the total power of all the extended votes (this will not always be true)

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/checkpointing/proposal.go#L101

https://github.com/sherlock-audit/2024-12-babylon/blob/main/babylon/x/checkpointing/proposal.go#L123-L124

Internal Pre-conditions

The vote extensions of inactive validators are used for evaluation

External Pre-conditions

NIL

Attack Path

See POC section

Impact

  • Proposal preparation can be blocked since the this returns early with an error during the proposal preparation and since validators do not prepare any proposal successfully then no block is created as there will be no proposal to process by the other validators
  • Proposal processing can also be blocked

This vicious loop can proceed ad infinitum causing the chain to halt as no block is processed.

PoC

There 2 scenarios: For simplicity, consider 3 extendedVotes

extendedVotes = [
    { ValidatorX.Power: 50, VoteExtension: {...BlockHash: "abc"} }, // signed in block N-2
    { ValidatorY.Power: 30, VoteExtension: {...BlockHash: "xyz"} }, // signed in block N-1
    { ValidatorZ.Power: 10, VoteExtension: {...BlockHash: "xyz"} } // signed in block N-1
]

SCENARIO ONE An unbonded validator's (ValidatorX) vote is used

  • this current epoch is 5, and current block is N = 101 (which is also the FirstBlockHeight in this epoch)
  • ValidatorX signed the extension at block N - 2 = 99 in the 4th epoch and was unbonded (removed) in 100th block (becomes inactive)
  • ValidatorY and ValidatorZ signed the extension at N - 1 = 100 in the 49th epoch
236: 		// Encode the block hash using hex
237: 		blockHashes[hex.EncodeToString(bHash)] += vote.Validator.Power
  • on blockHashes[abc] = 50
  • blockHashes[xyz] = 30 + 10 = 40
  • totalPower = 90
243: 	// Find the block hash that has the maximum voting power committed to it
244: 	for blockHash, power := range blockHashes {
245: 		if power > maxPower {
246: 			resBlockHash = blockHash // abc is the block hash that has the maximum voting power committed to it
247: 			maxPower = power // 50
248: 		}
249: 	}
  • maxPower = 50
requiredVP := ((totalPower * 2) / 3) + 1;
requiredVP := (90 * 2 / 3) + 1 = 61
  • requiredVP = 61
  • 50 < 61
255: 	maxPower < requiredVP {
256: 		return nil, fmt.Errorf(
257: 			"insufficient cumulative voting power received to verify vote extensions; got: %d, expected: >=%d",
258: 			maxPower, requiredVP,
259: 		)
  • an error is returned and processing or preparation is returned early

Notice that if we had used only active validators (i.e ValidatorY and ValidatorZ), the maxPower would be 40 and requiredVP would be (40 * 2 / 3) + 1 and this will pass without issues

SCENARIO TWO Same as scenario 1 but ValidatorX is still active (bonded) in the current block, the problem will still arise following the calculations above if the cummullative voting power of the blocks in which the extensions under consideration where signed are such that no one of them has 2/3 of the total voting power of all the extensions Using the extendedVotes from above, maxPower will be 50 while required is 61 leading to maxPower < requiredVP and an error is returned

Mitigation

I dont have a trivial solution but I suggest the findLastBlockHash()

  • using only validators that are present in the current block (i.e they had not unbonded as at the previous block)
  • implemented such that if maxPower < requiredVP the blockhash of the block that has the highest voting power signed in it should still be chosen (this is to prevent early return with an error.