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

1 stars 0 forks source link

Dliteofficial - Unapproved clients could start earning Proposal submission and/or voting rewards #20

Closed sherlock-admin3 closed 4 months ago

sherlock-admin3 commented 4 months ago

Dliteofficial

medium

Unapproved clients could start earning Proposal submission and/or voting rewards

Summary

Clients who are registered but unapproved can start earning Proposal submission and/or voting rewards immediately they are registered as long as they facilitate a successful proposal and voting on proposals submitted.

Vulnerability Detail

Resulting from the upgrade of DAO Logic, client incentives is implemented to encourage clients to facilitate proposal submission and voting. To enjoy this incentive, the client has to facilitate a successful proposal and or be a conduit for the voting on one.

For a client to be able to enjoy this incentive, 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 Proposal submission and/or voting 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.

    function updateRewardsForAuctions(uint32 lastNounId) public whenNotPaused {

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

        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/more successful proposals or vote(s) on a proposal than they did.

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-admin2 commented 4 months ago

2 comment(s) were left on this issue during the judging contest.

WangAudit commented:

seems to be intended behaviour; if they're registered but not approved then there's no incentive for doing anything; therefore; to me it looks working fine and the goal of approval is to allow them withdraw

takarez commented:

valid; medium(4)