Case-sensitive address check allows for double signing
Description
The IsDuplicateSigner()
function is used to check whether a given address already exists within a list of signers. It does this by doing a string comparison, which is case sensitive.
func isDuplicateSigner(creator string, signers []string) bool {
for _, v := range signers {
if creator == v {
return true
}
}
return false
}
This function is used in CreateTSSVoter()
, which is the message handler for the MsgCreateTSSVoter
message. This message is used by validators to vote on a new TSS.
func (k msgServer) CreateTSSVoter(goCtx context.Context, msg *types.MsgCreateTSSVoter) (*types.MsgCreateTSSVoterResponse, error) {
// [ ... ]
if isDuplicateSigner(msg.Creator, tssVoter.Signers) {
return nil, sdkerrors.Wrap(sdkerrors.ErrorInvalidSigner, fmt.Sprintf("signer %s double signing!!", msg.Creator))
}
// [ ... ]
// this needs full consensus on all validators.
if len(tssVoter.Signers) == len(validators) {
tss := types.TSS{
Creator: "",
Index: tssVoter.Chain,
Chain: tssVoter.Chain,
Address: tssVoter.Address,
Pubkey: tssVoter.Pubkey,
Signer: tssVoter.Signers,
FinalizedZetaHeight: uint64(ctx.BlockHeader().Height),
}
k.SetTSS(ctx, tss)
}
return &types.MsgCreateTSSVoterResponse{}, nil
}
Impact
In Cosmos-based chains, addresses are alphanumeric, and the alphabetical characters in the address can either be all uppercase or all lowercase when represented as a string. This means that case-sensitive string comparisons, such as the one in IsDuplicateSigner()
, can allow a single creator
to pass the check twice — once for an all lowercase address, and once for an all uppercase version of the same address.
Due to the len(tssVoter.Signers) == len(validators)
check, it is possible for a malicious actor to spin up multiple bonded validators and double sign with each of them. This would cause the check to erroneously pass, even though full consensus has not been reached, and allow the malicious actor to effectively force the vote to pass.
Recommendations
The sdk.AccAddressFromBech32()
function can be used to convert a string
address to an instance of a sdk.AccAddress
type. Comparing two sdk.AccAddress
types is the correct way to compare addresses in Cosmos-based chains, and it will fix this issue.
Remediation
This issue has been acknowledged by ZetaChain, and a fix was implemented in commit 83d0106b↗.