29095 - [SC - High] The lockers supply can be arbitrarily inflated ...

Submitted on Mar 7th 2024 at 00:56:31 UTC by @Trust for Boost | ZeroLend

Report ID: #29095

Report type: Smart Contract

Report severity: High

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

Impacts:

• Theft of unclaimed yield

Description

Brief/Intro

Users can lock Zero tokens in locker contracts (LockerLP/LockerToken) to receive locker NFTs. They can merge those NFTs through merge(). OmnichainStaking

Vulnerability Details

BaseLocker manages the current locked supply of underlying: uint256 public supply;

Supply is added in each _depositFor():

function _depositFor(
    uint256 _tokenId,
    uint256 _value,
    uint256 _unlockTime,
    LockedBalance memory _lock,
    DepositType _type
) internal {
    LockedBalance memory lock = _lock;
    uint256 supplyBefore = supply;
    supply = supplyBefore + _value;
    LockedBalance memory oldLocked;
    (oldLocked.amount, oldLocked.end, oldLocked.power) = (
        lock.amount,
        lock.end,
        lock.power
    );

It is likewise reduced in withdraw():

     uint256 supplyBefore = supply;
     supply = supplyBefore - value;

The issue is that not all _depositFor() calls increase the supply - when it is called for MERGE_TYPE deposit, it simply combines two existing supplies. Therefore, supply variable will become forever out of sync. It is manipulable by an attacker by creating two locks and merging them, he could do this at the last second of every week so that his founds would be blocked for only a single block.

uint256 unlockTime = ((block.timestamp + _lockDuration) / WEEK) * WEEK; // Locktime is rounded down to weeks
require(_value > 0, "value = 0"); // dev: need non-zero value
require(unlockTime > block.timestamp, "Can only lock in the future");

Impact Details

Anything that accounts using the locker supply() can be manipulated, for example to lead to a diluted reward distribution for a particular chain. Attacker can also make supply drift to a very large number, so that future deposits would overflow, making the contract unable to service createLock() requests.

Proof of Concept

THe POC is a standalone file including all the necessary contracts. Simply deploy BaseLockerPOC and run inflate_supply() to see that merging leaks supply() by the attacker.

// SPDX-License-Identifier: MIT
pragma solidity ^0.8.20;

import {IERC20} from "@openzeppelin/contracts/interfaces/IERC20.sol";
import "@openzeppelin/contracts/token/ERC20/ERC20.sol";

import {IERC721Receiver} from "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
import {ReentrancyGuard} from "@openzeppelin/contracts/utils/ReentrancyGuard.sol";
import {IERC165, ERC721EnumerableUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC721/extensions/ERC721EnumerableUpgradeable.sol";
import {ReentrancyGuardUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/ReentrancyGuardUpgradeable.sol";

import {IERC721} from "@openzeppelin/contracts/interfaces/IERC721.sol";
import {IERC721Receiver} from "@openzeppelin/contracts/interfaces/IERC721Receiver.sol";
import {IVotes} from "@openzeppelin/contracts/governance/utils/IVotes.sol";
import {OApp} from "@layerzerolabs/lz-evm-oapp-v2/contracts/oapp/OApp.sol";
import {Votes} from "@openzeppelin/contracts/governance/utils/Votes.sol";
interface IOmnichainStaking is IVotes {
    struct StakeInformation {
        address owner;
        uint256 tokenStake;
        uint256 lpStake;
        uint256 localVe;
    }

    // An omni-chain staking contract that allows users to stake their veNFT
    // and get some voting power. Once staked the voting power is available cross-chain.

    function unstakeLP(uint256 tokenId) external;

    function unstakeToken(uint256 tokenId) external;

    /// @dev using layerzero, sends the updated voting power across the different chains
    function updatePowerOnChain(uint256 chainId, uint256 nftId) external;

    /// @dev using layerzero, deletes the updated voting power across the different chains
    function deletePowerOnChain(uint256 chainId, uint256 nftId) external;

    /// @dev send the veStaked supply to the mainnet
    function updateSupplyToMainnetViaLZ() external;

    /// @dev receive the veStaked supply on the mainnet
    function updateSupplyFromLZ() external;
}


import {IERC721Enumerable} from "@openzeppelin/contracts/token/ERC721/extensions/IERC721Enumerable.sol";

interface ILocker is IERC721Enumerable {
    function balanceOfNFT(uint256 _tokenId) external view returns (uint256);

    function balanceOfNFTAt(
        uint256 _tokenId,
        uint256 _t
    ) external view returns (uint256);
}


import {ERC20VotesUpgradeable} from "@openzeppelin/contracts-upgradeable/token/ERC20/extensions/ERC20VotesUpgradeable.sol";

interface IZeroLocker is IERC721 {
    function balanceOfNFT(uint256) external view returns (uint256);

    function merge(uint256 _from, uint256 _to) external;

    function depositFor(uint256 _tokenId, uint256 _value) external;

    function createLockFor(
        uint256 _value,
        uint256 _lockDuration,
        address _to,
        bool _stakeNFT
    ) external returns (uint256);

    function createLock(
        uint256 _value,
        uint256 _lockDuration,
        bool _stakeNFT
    ) external returns (uint256);

    enum DepositType {
        DEPOSIT_FOR_TYPE,
        CREATE_LOCK_TYPE,
        INCREASE_LOCK_AMOUNT,
        INCREASE_UNLOCK_TIME,
        MERGE_TYPE
    }

    struct LockedBalance {