Skip to content

Latest commit

 

History

History
380 lines (331 loc) · 11.6 KB

Dup1337-Q.md

File metadata and controls

380 lines (331 loc) · 11.6 KB
Raw
Issues
[L-01] Wrong fee amount check doesn't protect against extensive fees
[L-02] Allowed spread check allows for smaller spread than expected
[L-03] No validation of token being removed has shares
[L-04] Opening nonleveraged position in pendle power farm is impossible
[L-05] Adding too much Pendle markets DoSes Pendle Power Farm
[L-06] Borrow APY calculations don't account for utilization rate
[L-07] Possible DoSing of Pendle farm via overflow
[NC-01] exitFarm doesn´t default the isAave flag

[L-01] Wrong fee amount check doesn't protect against extensive fees

Function PendlePowerFarmToken.depositExactAmount() checks if position mint fee is not too big by reverting with TooMuchFee() error, if reducedShares == feeShares evaluates to true. This check is not sufficient, because if feeShares are bigger than reducedShares, the condiiton will succeed. Hence, the protection is not sufficient to protect against too big fee.

PendlePowerFarmToken.sol
    function depositExactAmount(
//[...]
        uint256 reducedShares = _applyMintFee(
            shares
        );

        uint256 feeShares = shares
            - reducedShares;

        if (feeShares == 0) {
            revert ZeroFee();
        }

        if (reducedShares == feeShares) { // @audit it doesn't concern case when feeShares > reducedShares
            revert TooMuchFee();
        }

The check should be changed to if (reducedShares <= feeShares).

[L-02] Allowed spread check allows for smaller spread than expected

Allowed spread check allows for smaller spread than expected, because spread percentage is calculated from value after swaps, not before.

        uint256 ethValueBefore = _getTokensInETH(
            ENTRY_ASSET,
            _depositAmount
        );

        (
            uint256 receivedShares
            ,
        ) = IPendleChild(PENDLE_CHILD).depositExactAmount(
            netLpOut
        );

        uint256 ethValueAfter = _getTokensInETH( 
            PENDLE_CHILD,
            receivedShares
        )
            * _allowedSpread // @audit spread is calculated from diminished value
            / PRECISION_FACTOR_E18;

        if (ethValueAfter < ethValueBefore) {
            revert TooMuchValueLost();
        }

So, the check is actually if value ETH after * _allowedSpread >= deposit ETH value. However, the calculation checks if the spread is actually lower than set by user, because the slippage is applied to already diminished value. For example, let's say that user passes the following:

depositAmount = 100
allowedSlippage = 105e16 (5%)

So slippage down to 95 should be accepted. However, due to the flipped calculations, the slippage check would look like 95 * 105% < 100 => 99.75 < 100 and would revert, even though it should be accepted.

[L-03] No validation of token being removed has shares

The manual remove function doesnt validate whether the pool being removed has shares. It can be organicly have shares during the removal either by TX order or already the share is there;

Contract: FeeManager.sol

438:     function removePoolTokenManual(
439:         address _poolToken
440:     )
441:         external
442:         onlyMaster
443:     {
444:         uint256 i;
445:         uint256 len = getPoolTokenAddressesLength();
446:         uint256 lastEntry = len - 1;
447:         bool found;
448: 
449:         if (poolTokenAdded[_poolToken] == false) {
450:             revert PoolNotPresent();
451:         }
452: 
453:         while (i < len) {
454: 
455:             if (_poolToken != poolTokenAddresses[i]) {
456: 
457:                 unchecked {
458:                     ++i;
459:                 }
460: 
461:                 continue;
462:             }
463: 
464:             found = true;
465: 
466:             if (i != lastEntry) {
467:                 poolTokenAddresses[i] = poolTokenAddresses[lastEntry];
468:             }
469: 
470:             break;
471:         }
472: 
473:         if (found == true) {
474: 
475:             poolTokenAddresses.pop();
476:             poolTokenAdded[_poolToken] = false;
477: 
478:             emit PoolTokenRemoved(
479:                 _poolToken,
480:                 block.timestamp
481:             );
482: 
483:             return;
484:         }
485: 
486:         revert PoolNotPresent();
487:     }

[L-04] Opening nonleveraged position in pendle power farm is impossible

If user wants to have <100% exposure it will revert, in case if 1x leverage flash will be 0 and balancer will revert too

Contract: PendlePowerFarm.sol

185:     function _openPosition(
186:         bool _isAave,
187:         uint256 _nftId,
188:         uint256 _initialAmount,
189:         uint256 _leverage,
190:         uint256 _allowedSpread
191:     )
192:         internal
193:     {
194:         if (_leverage > MAX_LEVERAGE) {
195:             revert LeverageTooHigh();
196:         }
197: 
198:         uint256 leveragedAmount = getLeverageAmount(
199:             _initialAmount,
200:             _leverage
201:         );
202: 
203:         if (_notBelowMinDepositAmount(leveragedAmount) == false) {
204:             revert AmountTooSmall();
205:         }
206: 
207:         _executeBalancerFlashLoan(
208:             {
209:                 _nftId: _nftId,
210:                 _flashAmount: leveragedAmount - _initialAmount, // @audit if user wants to have <100% exposure it will revert, in case if 1x leverage flash will be 0 and balancer will revert too
211:                 _initialAmount: _initialAmount,
212:                 _lendingShares: 0,
213:                 _borrowShares: 0,
214:                 _allowedSpread: _allowedSpread,
215:                 _ethBack: false,
216:                 _isAave: _isAave
217:             }
218:         );
219:     }
220:

[L-05] Adding too much Pendle markets DoSes Pendle Power Farm

There is no constraint on how many pendle markets can be added:

    function addPendleMarket(
        address _pendleMarket,
        string memory _tokenName,
        string memory _symbolName,
        uint16 _maxCardinality
    )
        external
        onlyMaster
    {
// [...]
        activePendleMarkets.push(
            _pendleMarket
        );
// [...]

Adding too much doses syncing supply:

    function syncAllSupply()
        public
    {
        uint256 i;
        uint256 length = activePendleMarkets.length;

        while (i < length) {
            _syncSupply(
                activePendleMarkets[i]
            );
            unchecked {
                ++i;
            }
        }
    }

[L-06] Borrow APY calculations don't account for utilization rate

There are 2 functions calculating APY - one calculates borrow APY, one lending APY. So, generally both should yield similar results, that is lending APY - protocol fees ~= borowing APY. However lending APY includes utilization rate of the pool and APY is adjusted over it, while borrowing APY doesn't account for it, leading to borrowing APY assuming utilization rate is always 100%

    function overallLendingAPY(
        uint256 _nftId
    )
        external
        view
        returns (uint256)
    {
            weightedRate += ethValue
                * getLendingRate(token);

            overallETH += ethValue;

            unchecked {
                ++i;
            }
        }

        return weightedRate
            / overallETH;

and lendingRate() function:

getLendingRate(
        address _poolToken
    )
        public
        view
        returns (uint256)
    {
        uint256 pseudoTotalPool = WISE_LENDING.getPseudoTotalPool(
            _poolToken
        );

        if (pseudoTotalPool == 0) {
            return 0;
        }

        uint256 adjustedRate = getBorrowRate(_poolToken)
            * (PRECISION_FACTOR_E18 - WISE_LENDING.globalPoolData(_poolToken).poolFee)
            / PRECISION_FACTOR_E18;

        return adjustedRate // @audit pool utilization rate is taken into account
            * WISE_LENDING.getPseudoTotalBorrowAmount(_poolToken)
            / pseudoTotalPool;
    }

in comparison, borrow APY is calculated as follows:

    function overallBorrowAPY(
        uint256 _nftId
    )
        external
        view
        returns (uint256)
    {
        // [...]

            weightedRate += ethValue
                * getBorrowRate(token); // @audit borrow rate is WISE_LENDING.borrowPoolData(_poolToken).borrowRate storage variable

            overallETH += ethValue;

            unchecked {
                ++i;
            }
        }

        return weightedRate
            / overallETH;
    }

So borrow APY doesn't account for utilization rate as lending APY. So it shows that you'll have to pay high fees for borrowing, and you'll get proportionally less for providing value to the protocol.

There is actually yet another function, combining the two above - overallNetAPY(), which calculates both lending and borrowing APY, and returns the value combined.

Both functions are not used by the protocol, but it might false information to either offchain clients or external protocols integrating with WiseLending.

Additional thing to consider is that Pendle markets can be only added, there is no function to remove them.

[L-07] Possible DoSing of Pendle farm via overflow

there are 3 multiplications of big numbers before first division in PendleChildLpOracle:

function latestAnswer()
        public
        view
        returns (uint256)
    {
        return priceFeedPendleLpOracle.latestAnswer()
            * pendleChildToken.totalLpAssets()
            * PRECISION_FACTOR_E18
            / pendleChildToken.totalSupply()
            / PRECISION_FACTOR_E18;
    }

When I put some numbers, we still have a space to grow:

> 2**256 / (1e18*1000000e18*1e18)
115792089237316180

But either way, the protocol still can use MathLib.MulDiv that handles intermediate overflow gracefully in case of black swan events.

[NC-01] exitFarm doesn´t default the isAave flag

When the exitFarm is called, The Power Farm NFT is burned, Reserved keys are reset, And available NFT mapping is reserved for the burned one. But, it doesnt' revert the isAave mapping to false

Contract: PendlePowerManager.sol

210:     function exitFarm(
211:         uint256 _keyId,
212:         uint256 _allowedSpread,
213:         bool _ethBack 
214:     )
215:         external
216:         updatePools
217:         onlyKeyOwner(_keyId)
218:     {
219:         uint256 wiseLendingNFT = farmingKeys[
220:             _keyId
221:         ];
222: 
223:         delete farmingKeys[
224:             _keyId
225:         ];
226: 
227:         if (reservedKeys[msg.sender] == _keyId) {
228:             reservedKeys[msg.sender] = 0;
229:         } else {
230:             FARMS_NFTS.burnKey(
231:                 _keyId
232:             );
233:         }
234: 
235:         availableNFTs[
236:             ++availableNFTCount
237:         ] = wiseLendingNFT;
238: 
239:         _closingPosition(
240:             isAave[_keyId],//@audit if this remains as True, it will remain true
241:             wiseLendingNFT,
242:             _allowedSpread,
243:             _ethBack
244:         );
245: 
246:         emit FarmExit(
247:             _keyId,
248:             wiseLendingNFT,
249:             _allowedSpread,
250:             block.timestamp
251:         );
252:     }