Unapproved clients could start earning client auction rewards

Summary

Clients who are registered but unapproved can start earning auction rewards immediately they are registered as long as they facilitate the winning bid.

Vulnerability Detail

Per the new upgrade of NounsAuctionHouse to NounsAuctionHouseV2, client incentives is implemented to encourage clients to facilitate auction bids. To enjoy this incentive, the client has to facilitate the winning bid.

To be eligible, first, they have to register by calling Rewards::registerClient(). Calling this function doesnt approve the client, the DAO still has to give the stamp of approval. Unlike unapproved clients, approved clients are not just entitled to auction rewards, they can also withdraw the rewards garnered.

The vulnerability here arises from the ability of unapproved clients to claim rewards by posting the winning bid. Although this doesnt cause any financial loss to the protocol, it however, renders the approval process inefficient if a client doesnt necessary need approval to start earning rewards.

    Rewards::updateRewardsForAuctions()

    function updateRewardsForAuctions(uint32 lastNounId) public whenNotPaused {

        .................

 >>>       for (uint256 i; i < settlements.length; ++i) {
            INounsAuctionHouseV2.Settlement memory settlement = settlements[i];
            if (settlement.clientId != 0 && settlement.clientId <= maxClientId) {
                sawValidClientId = true;
                m.inc(settlement.clientId, settlement.amount);
            }
        }

        uint16 auctionRewardBps = $.params.auctionRewardBps;
        uint256 numValues = m.numValues();
        for (uint32 i = 0; i < numValues; ++i) {
            ClientRewardsMemoryMapping.ClientBalance memory cb = m.getValue(i);
            uint256 reward = (cb.balance * auctionRewardBps) / 10_000;
            $._clientMetadata[cb.clientId].rewarded += SafeCast.toUint96(reward);

            emit ClientRewarded(cb.clientId, reward);
        }

        ................
    }

Impact

As mentioned earlier, there is no financial loss to the protocol because they will be unable to withdraw their rewards until approval is granted, but unapproved clients will be denying approved clients of actually getting a reward because an unapproved client facilitated a bid that was above theirs.

Code Snippet

    function registerClient(string calldata name, string calldata description) external whenNotPaused returns (uint32) {
        RewardsStorage storage $ = _getRewardsStorage();

        uint32 tokenId = $.nextTokenId;
        $.nextTokenId = tokenId + 1;
        _mint(msg.sender, tokenId);

        ClientMetadata storage md = $._clientMetadata[tokenId];
        md.name = name;
        md.description = description;

        emit ClientRegistered(tokenId, name, description);

        return tokenId;
    }

Tool used

Manual Review

Recommendation

Only approved clients should be allowed to compete for client rewards.

sherlock-audit / 2024-03-nouns-dao-2-judging

Dliteofficial - Unapproved clients could start earning client auction rewards #19