[METER-LSD-03] Lack of Bucket Owner Verification in Updating Bucket Candidates

Impact

A malicious user can change the candidates of different buckets on the mainnet in bulk. This issue allows a malicious user to steal all the Block Rewards.

Description

The meter chain provides a path through the precompile contract via ScriptEngine.sol. Here's the code for the endpoint that sets up the candidate for the bucket.

{"native_bucket_update_candidate", func(env *xenv.Environment) []interface{} {
            var args struct {
                Owner            meter.Address
                BucketID         meter.Bytes32
                NewCandidateAddr meter.Address
            }
            env.ParseArgs(&args)
            env.UseGas(meter.GetBalanceGas)
            err := MeterTracker.Native(env.State()).BucketUpdateCandidate(args.Owner, args.BucketID, args.NewCandidateAddr)
            if err != nil {
                return []interface{}{err.Error()}
            }

            return []interface{}{""}
        }},

func (e *MeterTracker) BucketUpdateCandidate(owner meter.Address, id meter.Bytes32, newCandidateAddr meter.Address) error {
    candidateList := e.state.GetCandidateList()
    bucketList := e.state.GetBucketList()

    b := bucketList.Get(id)
    if b == nil {
        return errBucketNotListed
    }

    nc := candidateList.Get(newCandidateAddr)
    if nc == nil {
        return errCandidateNotListed
    }

    c := candidateList.Get(b.Candidate)
    // subtract totalVotes from old candidate
    if c != nil {
        if c.TotalVotes.Cmp(b.TotalVotes) < 0 {
            return errNotEnoughVotes
        }
        c.TotalVotes.Sub(c.TotalVotes, b.TotalVotes)
    }
    // add totalVotes to new candidate
    nc.TotalVotes.Add(nc.TotalVotes, b.TotalVotes)
    b.Candidate = nc.Addr

    e.state.SetBucketList(bucketList)
    e.state.SetCandidateList(candidateList)
    return nil
}

The code should only allow the bucket's owner to modify the candidate, but there is no code to check if the transaction sender is the bucket owner.

Therefore, it was possible to modify the candidate of every existing bucket and steal all the block rewards.

Recommendations

We recommend adding code to check if the Transaction Sender is the Bucket Owner.

meterio / sys-contracts