Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing Validation of Priorities in Validateset in Header and Many Other Known Security Issues in Cometbft are Unsolved #35

Open
Hellobloc opened this issue Oct 18, 2024 · 2 comments

Comments

@Hellobloc
Copy link

The header hash was generated with a missing hash of the validateset's priorities information. A malicious user could modify the priorities without causing a state hash validation error. Remarkably this is a known issue in Cometbft that breaks the state hash validation for priorities.

func (vals *ValidatorSet) Hash() []byte {
bzs := make([][]byte, len(vals.Validators))
for i, val := range vals.Validators {
bzs[i] = val.Bytes()
}
return merkle.HashFromByteSlices(bzs)
}

/types/validator.go
type Validator struct {
	Address     Address       `json:"address"`
	PubKey      crypto.PubKey `json:"pub_key"`
	VotingPower int64         `json:"voting_power"`

	ProposerPriority int64 `json:"proposer_priority"`
}
...
types/validator_set.go
func (vals *ValidatorSet) Hash() []byte {
	bzs := make([][]byte, len(vals.Validators))
	for i, val := range vals.Validators {
		bzs[i] = val.Bytes()
	}
	return merkle.HashFromByteSlices(bzs)
}
...
/types/validator.go
func (v *Validator) Bytes() []byte {
	pk, err := ce.PubKeyToProto(v.PubKey)
	if err != nil {
		panic(err)
	}

	pbv := cmtproto.SimpleValidator{
		PubKey:      &pk,
		VotingPower: v.VotingPower,
	}//missing ProposerPriority

	bz, err := pbv.Marshal()
	if err != nil {
		panic(err)
	}
	return bz
}

This project implemented its own consensus protocol using cometbft's fork project, but many of the flaws that were fixed in cometbft were not fixed by that project, and this issue is one of them.
More information is shown below:
Other Unsolved issues' Fix PR and Commits:
cometbft#3984
cometbft#3369
cometbft@d766d20
cometbft#890
cometbft#865

@Hellobloc
Copy link
Author

@adamfraser @BrendanChou

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

1 participant