29198 - [SC - Medium] Griefing attack to cause the rewards of a user ...

Griefing attack to cause the rewards of a user to be locked and when users claim the reward after maturity date, user will suffer the penalty.

Submitted on Mar 10th 2024 at 11:49:33 UTC by @perseverance for Boost | ZeroLend

Report ID: #29198

Report type: Smart Contract

Report severity: Medium

Target: https://github.com/zerolend/governance

Impacts:

Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)

Description

Griefing attack to cause the rewards of a user to be locked and when users claim the reward after maturity date, user will suffer the penalty.

Brief/Intro

VestedZeroNFT is a NFT based contract to hold all the user vests. NFTs can be traded on secondary marketplaces like Opensea, can be split into smaller chunks to allow for smaller otc deals to happen in secondary markets.

When mint a NFT tokenIT for a user, the function mint() can be used

https://github.com/zerolend/governance/blob/main/contracts/vesting/VestedZeroNFT.sol#L63-L72

function mint(
        address _who,
        uint256 _pending,
        uint256 _upfront,
        uint256 _linearDuration,
        uint256 _cliffDuration,
        uint256 _unlockDate,
        bool _hasPenalty,
        VestCategory _category
    ) external returns (uint256)

If the _hasPenalty is true, then when users claim, the the zero token of the ownerOf(id) is deducted and amount is toClaim

https://github.com/zerolend/governance/blob/main/contracts/vesting/VestedZeroNFT.sol#L170-L171

 uint256 _penalty = penalty(id);
toClaim += lock.pending - _penalty;

The _penalty is https://github.com/zerolend/governance/blob/main/contracts/vesting/VestedZeroNFT.sol#L207-L212

/// @inheritdoc IVestedZeroNFT
    function penalty(uint256 tokenId) public view returns (uint256) {
        LockDetails memory lock = tokenIdToLockDetails[tokenId];
        // (, uint256 _pending) = claimable(id);
        // TODO
        return (lock.pending * 5) / 10;
    }

https://github.com/zerolend/governance/blob/main/contracts/vesting/VestedZeroNFT.sol#L159-L198

function claim(
        uint256 id
    ) public nonReentrant whenNotPaused returns (uint256 toClaim) {
        require(!frozen[id], "frozen");

        LockDetails memory lock = tokenIdToLockDetails[id];

        if (lock.hasPenalty) {
            // if the user hasn't claimed before, then calculate how much penalty should be charged
            // and send the remaining tokens to the user
            if (lock.pendingClaimed == 0) {
                uint256 _penalty = penalty(id);
                toClaim += lock.pending - _penalty;
                lock.pendingClaimed = lock.pending;

                // send the penalty tokens back to the staking bonus
                // contract (used for staking bonuses)
                zero.transfer(stakingBonus, _penalty);
            }
        } else {
            (uint256 _upfront, uint256 _pending) = claimable(id);

            // handle vesting without penalties
            // handle the upfront vesting
            if (_upfront > 0 && lock.upfrontClaimed == 0) {
                toClaim += _upfront;
                lock.upfrontClaimed = _upfront;
            }

            // handle the linear vesting
            if (_pending > 0 && lock.pendingClaimed >= 0) {
                toClaim += _pending - lock.pendingClaimed;
                lock.pendingClaimed += _pending - lock.pendingClaimed;
            }
        }

        tokenIdToLockDetails[id] = lock;

        if (toClaim > 0) zero.transfer(ownerOf(id), toClaim);
    }

So the design of the protocol is, if users claim after the maturity date (unlockDate + LinearDuration), then users can claim without the penalty. This can be seen in the comment in line

https://github.com/zerolend/governance/blob/main/contracts/voter/gauge/RewardBase.sol#L86-L98

if (token == zero) {
            // if the token is ZERO; then vest it linearly for 3 months with a pentalty for
            // early withdrawals.
            vesting.mint(
                account, // address _who,
                _reward, // uint256 _pending,
                0, // uint256 _upfront,
                86400 * 30 * 3, // uint256 _linearDuration,
                0, // uint256 _cliffDuration,
                0, // uint256 _unlockDate,
                true, // bool _hasPenalty,
                IVestedZeroNFT.VestCategory.NORMAL // VestCategory _category
            );
        } else token.safeTransfer(account, _reward);

For a user that has aToken and varToken balance != 0, then when aTokenGauge and varToken gauge receive the reward, then the user also have some reward.

The user can receive the reward by calling the getReward function.

https://github.com/zerolend/governance/blob/main/contracts/voter/gauge/RewardBase.sol#L78-L100

// allows a user to claim rewards for a given token
    function getReward(
        address account,
        IERC20 token
    ) public nonReentrant updateReward(token, account) {
        uint256 _reward = rewards[token][account];
        rewards[token][account] = 0;

        if (token == zero) {
            // if the token is ZERO; then vest it linearly for 3 months with a pentalty for
            // early withdrawals.
            vesting.mint(
                account, // address _who,
                _reward, // uint256 _pending,
                0, // uint256 _upfront,
                86400 * 30 * 3, // uint256 _linearDuration,
                0, // uint256 _cliffDuration,
                0, // uint256 _unlockDate,
                true, // bool _hasPenalty,
                IVestedZeroNFT.VestCategory.NORMAL // VestCategory _category
            );
        } else token.safeTransfer(account, _reward);
    }

So if the reward token is zero then the contract will transfer the zero token to ZeroVestedNFT contract to mint a NFT token for the user. But notice that when minting the NFT token, the _hasPenalty is true.

The getReward is permissionless so anyone can call the getReward for another account.

Vulnerability Details

So if the zero rewards of a user is transfered to the ZeroVestedNFT, when claim after the LinearDuration time, the user still suffer the penalty for claimming.

The penalty amount is 50% of the pending reward. The rootcause is because the penalty calculation does not take into account the linearDuration. So the user always suffer the penalty that is 50% of the reward amount even the claim time is > unlockDate + LinearDuration

The _penalty is https://github.com/zerolend/governance/blob/main/contracts/vesting/VestedZeroNFT.sol#L207-L212

/// @inheritdoc IVestedZeroNFT
    function penalty(uint256 tokenId) public view returns (uint256) {
        LockDetails memory lock = tokenIdToLockDetails[tokenId];
        // (, uint256 _pending) = claimable(id);
        // TODO
        return (lock.pending * 5