code4rena-2023-06-angle-protocol-m04

[M-04] estimatedAPR() might return the wrong APR

보고서

Summary

APR을 잘못 계산하여 유저가 잘못된 정보를 얻게 된다.

Keyword

misinformation, bug, logic flaw

Vulnerability

SavingsVest.estimatedAPR는 현재 vestingProfitvestingPeriod를 이용하여 APR을 계산한다.

첫번째 문제는 vestingPeriod 가 1년이 되지 않더라도 1년을 기준으로 계산한다는 점이다. (이는 잘못 되었지만, 의도된 것으로 볼 수도 있다.)

    function estimatedAPR() external view returns (uint256 apr) {
        uint256 currentlyVestingProfit = vestingProfit;
        uint256 weightedAssets = vestingPeriod * totalAssets();
        if (currentlyVestingProfit != 0 && weightedAssets != 0)
            apr = (currentlyVestingProfit * 3600 * 24 * 365 * BASE_18) / weightedAssets;
    }
 
    function totalAssets() public view override returns (uint256) {
        return super.totalAssets() - lockedProfit();
    }
 
    /// @notice Amount of profit that are still vesting
    function lockedProfit() public view virtual returns (uint256) {
        // Get the last update and vesting delay.
        uint256 _lastUpdate = lastUpdate;
        uint256 _vestingPeriod = vestingPeriod;
 
        unchecked {
            // If the vesting period has passed, there is no locked profit.
            // This cannot overflow on human timescales
            if (block.timestamp >= _lastUpdate + _vestingPeriod) return 0;
 
            // Get the maximum amount we could return.
            uint256 currentlyVestingProfit = vestingProfit;
 
            // Compute how much profit remains locked based on the last time a profit was acknowledged
            // and the vesting period. It's impossible for an update to be in the future, so this will never underflow.
            return currentlyVestingProfit - (currentlyVestingProfit * (block.timestamp - _lastUpdate)) / _vestingPeriod;
        }
    }

SavingsVest.accrue 함수에서, 담보가 0.1% 이상 과담보 되었거나 저담보 되었을 때만 vestingProfitlastUpdate를 업데이트 한다.

따라서 lastUpdate는 매번 업데이트 되는 게 아니라 담보 비율인 collatRatio가 99.9% 이하거나 100.1% 이상인 경우에 업데이트 된다.

    function accrue() external returns (uint256 minted) {
        ...
        if (collatRatio > BASE_9 + BASE_6) {
            ...
            if (surplus != 0) {
                // Adding new profits relaunches to zero the vesting period for the profits that were
                // previously being vested
                vestingProfit = (lockedProfit() + surplus);
                lastUpdate = uint64(block.timestamp);
                _agToken.mint(address(this), surplus);
            }
        } else if (collatRatio < BASE_9 - BASE_6) {
            ...
            if (missing > currentLockedProfit) {
                vestingProfit = 0;
                missing = currentLockedProfit;
            } else {
                vestingProfit = currentLockedProfit - missing;
                lastUpdate = uint64(block.timestamp);
            }
            if (missing > 0) {
                _agToken.burnSelf(missing, address(this));
                _transmuter.updateNormalizer(missing, false);
            }
        }

여기서 또다른 문제는 vestingPeriod 가 끝난 경우이다. 베스팅 기간이 끝난 뒤에도 동일한 베스팅 비율을 사용하는 것은 잘못되었다.

  1. 처음에, vestingProfit = 100 이고 vestingPeriod = 10일 이라고 가정하자. 이 때의 estimatedAPR는 올바른 값을 리턴한다.
  2. 10일이 지난 뒤, 모든 vesting이 unlock 되었다. 하지만 collatRatio가 틀어지지 않아 accrue가 호출되지 않았다. lastUpdate는 동일하게 유지된다.
    1. lockedProfit() 값은 if (block.timestamp >= _lastUpdate + _vestingPeriod) return 0;에 의해 0이 된다.
  3. estimatedAPR에서 vestingProfit = 100 이고 vestingPeriod = 10일로 여전히 동일한 식으로 계산된 APR을 리턴할 것이다.
  4. 하지만 실제로는 더이상 locked profit이 없으므로, APR은 0이어야 맞다.

Impact

유저가 잘못된 정보를 기반으로 이율을 계산하게 된다.

Mitigation

vestingPeriod가 지나 lockedProfit가 0일 시 APR이 0을 리턴한다.

    function estimatedAPR() external view returns (uint256 apr) {
+       if (lockedProfit() == 0) return 0; //check locked profit first
 
        uint256 currentlyVestingProfit = vestingProfit;
        uint256 weightedAssets = vestingPeriod * totalAssets();
        if (currentlyVestingProfit != 0 && weightedAssets != 0)
            apr = (currentlyVestingProfit * 3600 * 24 * 365 * BASE_18) / weightedAssets;
    }

tags: bughunting, angle protocol, smart contract, solidity, stablecoin, logic flaw, severity medium