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

Limited Voting Options Allow Ballot Creation Spam #536

Open
c4-bot-4 opened this issue Dec 18, 2023 · 17 comments
Open

Limited Voting Options Allow Ballot Creation Spam #536

c4-bot-4 opened this issue Dec 18, 2023 · 17 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working insufficient quality report This report is not of sufficient quality M-05 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons

Comments

@c4-bot-4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/node/x/crosschain/keeper/keeper_cross_chain_tx_vote_inbound_tx.go#L95
https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/node/x/observer/keeper/msg_server_add_blame_vote.go#L39
https://github.com/code-423n4/2023-11-zetachain/blob/main/repos/node/x/observer/keeper/msg_server_add_block_header.go#L79

Vulnerability details

Impact

The message functions for certain types of voting allow only successful observation votes. This creates a problem if a compromised or faulty observer creates spam ballots with false observations. In this situation, honest observers have to spend resources processing the ballot but cannot cast an opposite vote to slash the spammer when the ballot reaches maturity.

Theoritically, a similar situation could happen with all ballots that do not reach the threshold. But in non-vulnerable cases, honest observer can cast an opposite vote to make the ballot reach threshold and be accounted for slashing. The real issue comes when honest observers cannot cast the honest opposite vote.

Proof of Concept

For certain types of votes, the only vote that is possible to be cast is an VoteType_SuccessObservation.

ballot, err = k.zetaObserverKeeper.AddVoteToBallot(ctx, ballot, msg.Creator, observerTypes.VoteType_SuccessObservation)
ballot, err = k.AddVoteToBallot(ctx, ballot, vote.Creator, types.VoteType_SuccessObservation)
ballot, err = k.AddVoteToBallot(ctx, ballot, msg.Creator, types.VoteType_SuccessObservation)

A compromised or faulty observer can then create ballots for spam observations that do not exist, and honest observers can never vote for the opposite option. Since the ballot's threshold will never be reached, the ballot will remain with the status BallotStatus_BallotInProgress.

Eventually, the ballot will be processed when the DistributeObserverRewards function gathers the list of matured ballots.

ballotIdentifiers := keeper.GetObserverKeeper().GetMaturedBallotList(ctx)

But, since BuildRewardsDistribution does not account for ballots with status BallotStatus_BallotInProgress, the ballot spammer will have a totalRewardUnits == 0 instead of a negative value. This way, no slashing will be executed as a punishment for the false observation, but observers will have to process the spam observation ballot anyway.

func (m Ballot) BuildRewardsDistribution(rewardsMap map[string]int64) int64 {
	totalRewardUnits := int64(0)
	switch m.BallotStatus {
	case BallotStatus_BallotFinalized_SuccessObservation:
// ... SNIP ...
	case BallotStatus_BallotFinalized_FailureObservation:
// ... SNIP ...
return totalRewardUnits

The compromised observer is then free to create as many false observation ballots as he wishes, effectively spamming the network and draining honest observer's resources without a punishment.

Additional Note

Besides wasting network participant's execution resources, the ballots are never deleted from storage after the distribution. But there is a comment evidencing this is an obviously known issue and, thus, included as merely a reminder in this report.

	// TODO : Delete Ballots after distribution
	// https://github.com/zeta-chain/node/issues/942

Tools Used

Manual: code editor.

Recommended Mitigation Steps

Similarly to what happens in other types of votes, allow honest observers to cast a negative observation vote if they wish. Thus, when the ballot reaches maturity and is processed for reward distribution, the dishonest or faulty observer is slashed to pay for any resources spent with the false observation.

This is how msg_tss_voter.go and keeper_cross_chain_tx_vote_outbound_tx.go work:

	if msg.Status == common.ReceiveStatus_Success {
		ballot, err = k.zetaObserverKeeper.AddVoteToBallot(ctx, ballot, msg.Creator, observerTypes.VoteType_SuccessObservation)
		if err != nil {
			return &types.MsgCreateTSSVoterResponse{}, err
		}
	} else if msg.Status == common.ReceiveStatus_Failed {
		ballot, err = k.zetaObserverKeeper.AddVoteToBallot(ctx, ballot, msg.Creator, observerTypes.VoteType_FailureObservation)
		if err != nil {
			return &types.MsgCreateTSSVoterResponse{}, err
		}
	}
	ballot, err = k.zetaObserverKeeper.AddVoteToBallot(ctx, ballot, msg.Creator, observerTypes.ConvertReceiveStatusToVoteType(msg.Status))
	if err != nil {
		return nil, err
	}

Special attention must be give to ensure the corrected voting type's Digest function output is generalised for all types of votes. Similarly to what is done with MsgVoteOnObservedOutboundTx.

func (msg *MsgVoteOnObservedOutboundTx) Digest() string {
	// ... SNIP ...
	// Set status to ReceiveStatus_Created to make sure both successful and failed votes are added to the same ballot
	m.Status = common.ReceiveStatus_Created
	// ... SNIP ...

Assessed type

DoS

@c4-bot-4 c4-bot-4 added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working labels Dec 18, 2023
c4-bot-1 added a commit that referenced this issue Dec 18, 2023
@DadeKuma
Copy link

QA

@c4-pre-sort
Copy link

DadeKuma marked the issue as insufficient quality report

@c4-pre-sort c4-pre-sort added the insufficient quality report This report is not of sufficient quality label Dec 19, 2023
@c4-pre-sort
Copy link

DadeKuma marked the issue as duplicate of #223

@c4-judge
Copy link

0xean marked the issue as unsatisfactory:
Insufficient quality

@c4-judge c4-judge added the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 10, 2024
@0xean
Copy link

0xean commented Jan 15, 2024

I agree there is no way to slash a malicious observer who adds spam ballots (that I can currently find), I am also uncertain why the other observers would need to "waste" any resources since they wouldn't vote (as a for vote is the only option). Given that ballots aren't removed ever, it does seem that the state could grow indefinitely with no cost to the attacker. Will ask for sponsor comment here.

@c4-judge
Copy link

0xean marked the issue as not a duplicate

@c4-judge
Copy link

0xean marked the issue as primary issue

@c4-judge c4-judge added the primary issue Highest quality submission among a set of duplicates label Jan 15, 2024
@c4-judge c4-judge reopened this Jan 15, 2024
@c4-judge c4-judge removed the unsatisfactory does not satisfy C4 submission criteria; not eligible for awards label Jan 15, 2024
@c4-judge
Copy link

0xean removed the grade

@c4-judge
Copy link

0xean marked the issue as selected for report

@brewmaster012
Copy link

I agree there is no way to slash a malicious observer who adds spam ballots (that I can currently find), I am also uncertain why the other observers would need to "waste" any resources since they wouldn't vote (as a for vote is the only option). Given that ballots aren't removed ever, it does seem that the state could grow indefinitely with no cost to the attacker. Will ask for sponsor comment here.

This is known issue and incentives to the observer sets are work in progress.

@0xean
Copy link

0xean commented Jan 16, 2024

thanks @brewmaster012 - so that I can close the issue, can you provide reference to the known issue, is it in previous audits or where is it documented?

@c4-sponsor
Copy link

lumtis (sponsor) acknowledged

@c4-sponsor c4-sponsor added the sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons label Jan 17, 2024
@brewmaster012
Copy link

thanks @brewmaster012 - so that I can close the issue, can you provide reference to the known issue, is it in previous audits or where is it documented?

Our incentives to observers/signers are not complete, so we rely on a permissioned set of observers/signers for now.
Assumption is that observers might be faulty but they are not malicious, especially there is no direct profit to be gained.

@0xean
Copy link

0xean commented Jan 17, 2024

thanks @brewmaster012 - where can I find this documented in the main docs or contest docs? I need some public reference to make this fair to the wardens before closing.

@brewmaster012
Copy link

thanks @brewmaster012 - where can I find this documented in the main docs or contest docs? I need some public reference to make this fair to the wardens before closing.

I thought this is part of threat modeling

@0xean
Copy link

0xean commented Jan 23, 2024

The threat modeling that was done as a part of C4 certainly highlighted the potential for observers to be part of potential threats, but I have looked many places for any documentation that states observers are trusted and therefore should be considered a Centralization Risk / Admin Privilege which would make this issue QA. As such, I am going to leave this as a valid M finding.

@c4-judge
Copy link

0xean marked the issue as satisfactory

@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jan 23, 2024
@C4-Staff C4-Staff added the M-05 label Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working insufficient quality report This report is not of sufficient quality M-05 primary issue Highest quality submission among a set of duplicates satisfactory satisfies C4 submission criteria; eligible for awards selected for report This submission will be included/highlighted in the audit report sponsor acknowledged Technically the issue is correct, but we're not going to resolve it for XYZ reasons
Projects
None yet
Development

No branches or pull requests

8 participants