Leveraged Volatility Farming

Proof of concept: PoC

The getPrice function should return price as (aspTKN / pairedLPToken) which is consumed by the Fraxlend isSolvent function to determine a borrower's LTV.

However, the calculation is incorrect because it takes price from the spTKN oracle and divides it by _aspTknPerSpTkn when it should be multiplying instead. Therefore, the price returned is always incorrect and borrower's LTV is miscalculated in Fraxlend.

Instead of _priceLow = (_priceLow * _assetFactor) / _aspTknPerSpTkn; do _priceLow = (_priceLow * _aspTknPerSpTkn) / _assetFactor;

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

Function _processRewardsToPodLp is called on every major flow such as deposit and withdrawal to compound any earned rewards to the LP token and back into the AutoCompoundingPodLp. The amounts passed to indexUtils.addLPAndStake will be the entire balance of POD in the AutoCompoundingPodLp, and half of the paired lp token that’s obtained from the reward tokens with a V3 swap.

The issue is that the token A and token B amounts can be wildly different from their current reserve ratio in the pool, causing the desired token inputs to fail the calculated amountAMin and amountBMin passed to Uniswap V2.

There can be a multitude of reasons why few paired LP tokens are to be added, such as small reward distribution since the last reward claim and/or V3 swap manipulation in swapV3Single since 0 slippage is passed. An attacker is not necessary for the UniV2 revert to occur.

Attackers can also inflate the balances with token donations to trigger this revert as well. This DoS will occur even if LP_SLIPPAGE was drastically increased. Ultimately, all core functionalities of the AutoCompoundingPodLp can be prevented, and users can lose assets due to the inability to withdraw.

In _pairedLpTokenToPodLp, consider performing some sanity checks to ensure tokens are in the correct ratio before calling IndexUtils.addLPAndStake. Furthermore, considering wrapping the auto-compounding in a try-catch.

Peapods Team: Resolved in the following code change.

Guardian Team: The implemented single-sided LP formula is incorrectly implemented, using _fullAmt instead of _r where necessary. This will make the result of _pairedSwapAmt larger than _amountIn, 21 causing an underflow and blocking all rewards processing.

Proof of concept: PoC

The price of a spTKN is affected by the amount and price of the underlying token in the pod. This is accounted for in _accountForCBRInPrice which does:

(_amtUnderlying * IERC20(_underlying).balanceOf(_pod) * 10 * IERC20Metadata(_pod).decimals()) / IERC20(_pod).totalSupply() / 10 * IERC20Metadata(_underlying).decimals();

The problem lies in using the balance of underlying the pod, which can be easily manipulated through a donation of the underlying token to the pod. This would increase the value of spTKN and subsequently the aspTKN, which is the collateral token in FraxPairLend.

Although the attacker loses the donated tokens, they can manipulate the oracle pricing to borrow more tokens from the lending protocol, exploiting the system. Low liquidity pods are more susceptible to such attacks.

Instead of using balanceOf, use a storage variable to keep track of the balance of underlying tokens in a pod.

Peapods Team: Resolved in the following code change.

When removing leverage, the user provides the _borrowAssetAmt to be flash loaned. This amount is then used to calculate _borrowSharesToRepay, with the calculation performed by rounding up.

Additionally, the LeverageManager grants approval to the Fraxlend pair for exactly the _borrowAssetAmt. Then, on the Fraxlend side, amount to repay is recalculated using this _borrowSharesToRepay.

However, this calculation also rounds up. _amountToRepay = _totalBorrow.toAmount(_shares, true);. Because of rounding up twice during this transaction flow, the _amountToRepay ends up being higher than the flash-loaned _borrowAssetAmt.

When the internal _repayAsset function attempts to transfer _amountToRepay from LeverageManager to Fraxlend pair, it fails because the LeverageManager neither holds that amount of the borrow asset nor has given that amount of approval to the Fraxlend pair.

Consider passing in the shares in removeLeverage and calculate the flash loan amount from the shares to mimic FraxLend logic.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

totalAvailableAssetsForVault should return the available assets that a FraxlendPair vault can pull from LendingAsssetVault.However, several accounting errors exist resulting in the under-calculation of available assets. Consider these two examples of a single whitelisted vault with 100% allocation:

LendingAssetVault has 10 DAI of which 6 DAI has been withdrawn into FraxPair Vault

Example 1

• totalAvailableAssetsForVault will return 0 when it should return 4 instead

Example 2

• LendingAssetVault has 10 DAI of which 4 DAI has been withdrawn into FraxPair Vault
• totalAvailableAssetsForVault will return (10 - 4) - 4 = 2 when it should return 6 instead

As FraxLendPair relies heavily on this function to obtain available assets from LendingAssetVault this results in: 1) preventing further whitelistWithdraw after 50% of assets are withdrawn, 2) inflating utilization rate in FraxlendPair and increase interest charged to borrowers, 3) deposits allowed above the depositLimit.

Update the function to:

uint256 _overallAvailable = totalAvailableAssets();
uint256 _vaultMax = ((_totalAssets * _vaultMaxPerc[_vault]) / PERCENTAGE_PRECISION);
uint256 _totalVaultAvailable = _vaultMax > vaultUtilization[_vault] ?
      _vaultMax - vaultUtilization[_vault] : 0;
_totalVaultAvailable = _overallAvailable < _totalVaultAvailable ?
      _overallAvailable : _totalVaultAvailable;
return _totalVaultAvailable;

Peapods Team: Resolved in the following code change.

The update function can be called by anyone to update another user's stake position. The issue lies in _update which burns a user's share balance if the CBR had decreased from the time when the user first staked.

Consider this example:

• Alice stakes 10 pTKNs and received 10 voting shares. CBR is 1.0
• CBR drops to 0.8 due to external factors
• Bob calls update on Alice's position. 2 shares are burned from her

If Bob was unable to call update on Alice's position, she could choose to do nothing and preserve her shares, potentially waiting for CBR to recover before performing more staking actions.

Furthermore, the CBR can be be drastically decreased with a flashloan from the pod, which would allow Bob to initially stake, flashloan to decrease CBR, and update Alice's position to burn all of her voting power.

Consequently, Bob can have all the voting power and claim all the rewards at the expense of other stakers.

Do not allow update to be called on another user's position and consider limiting direct balance checks. Also, do re-consider the design of the burn during _update as it will deter users from staking if their previous shares are burnt due to a change in CBR.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

The classic inflation attack during the first deposit is possible in the LendingAssetVault (LAV) through the donate function.

Attack scenario:

• LAV is created. attacker deposits 1 wei of assets and receives 1 share
• Attacker observes User depositing 100e18 of assets and frontruns with a donation of 100e18

assets

• User deposits 100e18 but receives 0 shares due to rounding down
• Attacker redeems 1 share and receives all assets in the vault (200e18 + 1 wei)
• User loses all deposits

Consider removing the donate function. Or else, consider other forms of protection against inflation attack, see https://blog.openzeppelin.com/a-novel-defense-against-erc4626-inflation-attacks

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

In a previous audit, it was reported that the aspTKN oracle could be manipulated through a donation of spTKN (see https://hackmd.io/@tapir/SyxqzohUA#H-9-aspTKN-oracle-can-be-manipulated).

While this has been fixed through the accounting of assets with a storage variable, this attack is still possible through donation of reward tokens which then gets compounded into spTKNs (assets).

_processRewardsToPodLp is called on every external user action, which checks for balanceOf reward tokens in the contract, and then converts the reward tokens to spTKNs.

Similar to the previously reported issue, although the attacker loses the donated tokens, they can manipulate the oracle pricing to borrow more tokens from the lending protocol, exploiting the system. Low liquidity pods are more susceptible to such attacks.

No straightforward solution as existing reward flow relies on transferring tokens to the AutoCompounder. Consider re-designing the reward and compounding flow to ensure the oracle pricing cannot be easily manipulated through a donation of reward tokens.

Peapods Team: Resolved in the following code change.

Guardian Team: If the intermediate token is also a reward token, then the entire balance would be transferred. This allows an attacker to donate that intermediate token, and it would bypass the maxSwap caps that were implemented to remedy the issue originally.

The getPrices function aims to return price in 18 decimals, which will be consumed by the aspTKN oracle and ultimately the FraxlendPair contract to determine LTV and borrow amount.

The issue lies in _calculateBasePerSpTkn where token decimals are correctly handled up till the calculation of _pairPrice18. As the variable name suggests, this is the price of the LP pair returned in 18 decimals:

uint256 _pairPrice18 = (2 * _avgBaseAssetInLp18 * 10 ** ((_clT0Decimals + _clT1Decimals) / 2)) / IERC20(_pair).totalSupply();

However, if either token is not in 18 decimals, e.g. USDC: 6 decimals, then the price returned here will not be in 18 decimals.Assume token0 is 18 decimals and token1 is 6 decimals, the math for decimals works out to be: 18 + ((18 + 6) / 2) - 18 = 12.

The incorrect precision affects all downstream calculations and results in a wrong price consumed by FraxlendPair. This ultimately affects LTV calculations which can lead to pairs being drained and users being liquidated unfairly.

Change the calculations to: uint256 _pairPrice18 = (2 * _avgBaseAssetInLp18 * 10 ** 18 / IERC20(_pair).totalSupply();

Afterwards, remove the _baseTDecimals logic in _spTknBasePrice18.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

UniswapDexAdapter uses IUniswapV2Router02.sol to initiate calls to the V2 Router. In this interface the swapExactTokensForTokensSupportingFeeOnTransferTokens functions is expected to return an array with amounts.

However, in the actual implementation of that function there are no values returned. This mismatch will result in a revert every time the function is called causing a DOS for the protocol.

Correct the interface to exclude the returned array.

Peapods Team: Resolved in the following code change.

In the function _updateInterestAndMdInAllVaults, which is called during every deposit/mint, addInterest() is called to trigger interest accrual on the FraxlendPair vault.

However, the wrong interface is used which should pass bool _returnAccounting as a function input. Therefore, the current implementation would always fail and DOS all deposits into LendingAssetVault.

Use the correct interface for addInterest.

Peapods Team: Resolved in the following code change.

If a smart contract has IFlashLoanRecipient::callback() or a fallback() function, a malicious user can set them as the receiver of the flashMint() function. This will burn .1% of their pTKN balance.

Since the only restriction on the amount of pTKNs minted is that the total supply does not overflow, this can allow a user to burn the entirety of the contract's pTKN balance.

This is profitable for the malicious user because burning pTKNs increases the value of existing pTKNs since they are now backed by more underlying tokens.

Charge the fee to the msg.sender instead of the _recipient.

Peapods Team: Resolved in the following code change.

The function redeemFromVault can be called by an attacker who passes in an arbitrary _vault and _amountShares

As this function can be called by anyone, a griefing attack is possible to call redeemFromVault to deny LendingAssetVault of yield (whenever there is available liquidity in FraxlendPair).

Validate the input data and consider only allowing owner to call redeemFromVault.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

Asset availability changes during whitelistDeposit and whitelistWithdraw. But, in these functions only the vault that is calling is updated with _updateAssetMetadataFromVault. Instead, all whitelisted vaults should be updated too which affects their interest calculations.

Consider this example:

• LAV has 100 DAI
• Two Vaults A & B, with a 100% and 50% max allocation from LAV respectively.
• Vault A & B each have whitelist withdrawn 25 DAI, so A's utilization rate is 25 / 75 = 33% while B's is

25 / 50 = 50%

• 1 day passes
• A new borrower borrows 50 DAI from Vault A, so utilization rate increases from 33 to 100%.
• Available assets are also reduced for vault B, its utilization rate will increase to 25 / 25 = 100%

In the example above, the next time accrued interest in Vault B is calculated, it assumes a 100% utilization rate for the entire duration since the last update.

Instead, it should have been a 50% utilization (lower interest rate) for 1 day and then the 100% utilization rate after the borrow from Vault A. Borrowers will therefore always be incorrectly charged for interest across all whitelisted vaults.

whitelistDeposit and whitelistWithdraw should update all vaults by calling both: _updateAssetMetadataFromVault(_vault) and _updateInterestAndMdInAllVaults(_vault)

Peapods Team: Resolved in the following code change.

Guardian Team: The recommendation was not implemented.

As there is no penalty nor timelock for unstaking with the StakingPoolToken, a user may claim rewards without actually staking by front-running depositReward and do: stake -> depositRewards -> unstake.

The user would immediately be eligible to claim rewards at the expense of other users who are staked.

Consider implementing a timelock or penalty for unstaking. Also, consider using time-weighted reward distribution.

Peapods Team: Acknowledged.

When compounding rewards in _processRewardsToPodLp, some rewards may require a two-step process to swap to the paired LP token.

However in _swapV2, only a single swap is performed. Therefore, if a second swap is required, it will not be performed and the intermediate token received will remain in the contract.

These rewards will not be compounded and users will lose potential yield. An identical error was found in Zapper.sol.

Perform a second swap for tokens which require a two-step process.

Peapods Team: Resolved in the following code change.

Users stake their LP tokens because spTokens accrue rewards in different tokens. The criteria for such a token is to either be a part of the whitelisted ones or be the specified token for the given TokenRewards contract.

When spTokens are deposited to aspTokens, the AutoCompondingPodLp contract receives the rewards and uses _processRewardsToPodLp to convert them to new spTokens. However, it does so only for the whitelisted tokens and ignores the specific reward token for the TokenRewards.

In result, any rewards accumulated in the specific token that is not part of the whitelisted tokens will not be correctly distributed to the holders of the aspTokens.

In addition to the whitelisted tokens collect the rewards from the rewardsToken as well.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

In LeverageManager, the addLeverage and removeLeverage functions allows an attacker to pass in a malicious contract as _selfLendingPairPod and _dexAdapter respectively.

This opens up the surface for attacks and reentrancy. It could be used for example to avoid payment of close fees during removeLeverage:

1. Alice calls removeLeverage passing in malicious contract as _dexAdapter
2. Malicious contract is called in _swapPodForBorrowToken
3. Malicious contract does the swap with a DEX but does not return any pod tokens, instead

transferring directly to Alice 4. As no pod tokens remain in LeverageManager, no close fees are applied at the end of callback and the protocol loses revenue.

Perform validation on the _selfLendingPairPod and _dexAdapter inputs. Do not allow users to provide arbitrary dexAdapter addresses, and use every WeightedIndex's own immutable dexAdapter.

Additionally, consider minimizing the use of arbitrary inputs in public functions, as they can expand the attack surface and introduce potential vulnerabilities, such as reentrancy risks

Peapods Team: Resolved in the following code change.

Guardian Team: Because there is no validation on _overrideLendingPair, users can pass in an arbitrary, malicious contract for this address. The malicious contract can return zero for the _podAmtRemaining, which the protocol relies upon to calculate the _closeFeeAmt.

When token rewards such as ARB are deposited into the TokenRewards contract, a large portion of it is expected to be transferred to AutoCompoundingPodLp which is a large holder of spTKNs.

Then the next time a user interacts with AutoCompoundPodLp, _processRewardsToPodLp is called to compound the reward tokens into more LP, benefiting existing holders of aspTKN.

As there is no penalty nor timelock for deposits and withdrawals, a user may front-run the deposit of reward tokens by depositing into AutoCompoundingPodLp so as to claim some of the rewards, and then back-run the deposit of rewards and withdraw from AutoCompoundingPodLp. Such extractive behavior results in less rewards for other users who are staked for the long term.

Peapods Team: Acknowledged.

When removing leverage, the amount of shares to repay to the Fraxlend vault is calculated based off the amount of the borrowed assets that are desired to be repaid. This is accomplished by calling toShares().

However, this calculation is done before any call is made to the Fraxlend vault. Because of this, the calculation of shares to be repaid does not account for any interest that has been accrued.

If enough time has passed or the interest rate is high enough, it will lead to an inaccurate amount of shares to pay off based on the amount of assets provided. This will lead to a revert in repayAssets(), due to insufficient balance and DoS removeLeverage()

Prior to the calculation of _borrowSharesToRepay in removeLeverage(), call addInterest() on the vault.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

During liquidation of a borrower, bad debt is only realized when the borrower has zero leftover collateral (see FraxlendPairCore.sol: 1130). This allows a liquidator to liquidate just enough shares such that a dust amount of collateral is left behind.

Thereafter, there might be little to no incentive for other liquidators to liquidate the borrower as the gas cost to do so exceeds the collateral value.

As a result, the bad debt is not socialized and lenders may exit the system without any losses. The liquidator himself may be a lender and therefore incentivized to exploit this loophole.

Consider implementing a threshold for collateral remaining after liquidation, such that a liquidator must leave sufficient collateral behind if doing a partial liquidation.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

When the Fraxlend pair does not have enough assets to lend, necessary amounts are transferred from the LendingAssetVault(LAV), and Frax shares are minted to LAV. The opposite occurs when removing leverage: Frax shares are burned from the LAV, and assets are transferred back to the vault.

The share amounts to mint and burn are always in favor of the protocol. Frax shares that the LAV receives during borrowing are rounded down in _depositFromVault, while Frax shares burned during repayment are rounded up in _withdrawToVault, as expected.

In the _repayAsset function, the _withdrawToVault is called with the asset amounts to repay. However, this causes DoS in certain situations. When attempting to transfer the entire utilized amount back (_extAmount == _externalAssetsToWithdraw), the share amount is rounded up in _withdrawToVault, causing the function to revert due to the insufficient balance error, as the LAV holds 1 fewer shares.

Check the share balance of the LAV before burning, and burn shares up to LAV balance.

Peapods Team: Resolved in the following code change.

borrowAsset and repayAsset functions may be paused by admin but can be bypassed through leveragePosition and repayAssetWithCollateral which performs borrow/repay actions. This loophole could be exploited by attackers while the protocol is paused.

Consider extending the pause effects to the leveragePosition and repayAssetWithCollateral functions.

Peapods Team: Resolved in the following code change.

When calling addLeverage(), a user is allowed to input whatever _selfLendingPairPod they desire, without any validation. This will allow a user to successfully add leverage with a self lending pod that is different from the pod they used to create their position.

However when they attempt to withdraw, they will be forced to use the pod associated with their NFT. This will prevent a user from removing leverage on their position, and force the position to be open indefinitely.

Since positions are transferable, a malicious user could sell their position to an unsuspecting user. This will lead to a user being stuck with a worthless position.

Validate that the _selfLendingPairPod passed in to addLeverage() is the same pod associated with their NFT. Alternatively, allow users to update the selfLendingPod they have associated with their position NFT.

Peapods Team: Resolved in the following code change.

WeightedIndex.flashMint() allows anyone to sandwich protocol actions by manipulating totalSupply().

There are a lot of parts in the protocol that depend on totalSupply:

• WeightedIndex.convertToShares()
• WeightedIndex.convertToAssets()
• ConversionFactorPTKN._calculateCbrWithDen()

Either change the code that depends on totalSupply or reconsider the existence of the flashMint function.

Peapods Team: Resolved in the following code change.

When compounding rewards in _pairedLpTokenToPodLp, pairedLpTokens are swapped to pTKN via Uniswap's swapV2Single. However, if too little tokens are provided it may revert in Uniswap V2 with 'INSUFFICIENT_INPUT_AMOUNT' or INSUFFICIENT_OUTPUT_AMOUNT .

This could be caused by a balance of 1 wei of pairedLpTokens. which after halving becomes 0 pTKNs. This small balance could be easily donated by an attacker looking to DOS the contract, or simply caused by leftover tokens from a previous transaction.

As processing of rewards is called by every major flow, reverting could have serious implications such as preventing users from removing leverage and result in liquidations.

Verify the balance of tokens before calling the swap function to avoid reverts.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

Whenever _updateAssetMetadataFromVault is called, a vault's Collateral Backed Ratio (CBR) is updated and compared against its previous value. If the CBR decreased from the previous update, then the vault's utilization is also decreased based on _vaultAssetRatioChange.

The issue occurs when _vaultAssetRatioChange is greater than 100%. This leads to an underflow when updating vaultUtilization[_vault].

Consider this example:

• Vault's CBR decreased from 100e27 to 49e27
• _vaultAssetRatioChange: (100e27 * 1e27 / 49e27 ) - 1e27 = 1.04e27
• vaultUtilization[_vault]: 100 - (100 * 1.04e27 / 1e27) = underflow

Such a drastic drop in CBR is unlikely but possible in vaults with obscure tokens (as Peapods is designed to be used permissionlessly). A revert in the update for one vault will cause DOS in all whitelisted vaults, and prevent liquidations in FraxlendPair vaults.

Handle the case when _vaultAssetRatioChange is greater than 100% to avoid the underflow.

Peapods Team: Resolved in the following code change.

When a user receives shares from TokenRewards for the first time, _cumulativeRewards is called to update the user's rewards mapping with an excluded amount. This amount will be used to calculated the user's share of future rewards.

The issue occurs in _cumulativeRewards: (_share * _rewardsPerShare[_token]) / PRECISION

By rounding down the calculation, it is essentially excludes the user from 1 wei less of rewards, implying the user earned 1 wei more rewards. This will overtime lead to insufficient balance of rewards to transfer out, and DOS all functionality of the contract when that happens.

Always round up the calculation in _cumulativeRewards when calculating the excluded amount.

Peapods Team: Resolved in the following code change.

When a new position is initialized, its lendingPair is set to the lending pair for the pod configured by the owner of the contract. However, adding and removing leverage always use the most recent lendingPair for the pod of the position instead of the pair at the time of it creation.

This results in positions being lost when the owner changes the pair by calling setLendingPair because _removeLeverage will make the custodian remove collateral from the new pair where it doesn't have any.

Use positionProps.lendingPair instead of the latest lending pair when adding/removing leverage.

Peapods Team: Resolved in the following code change.

When adding leverage, the user provides pod tokens, and the corresponding pairedLpTokens are flash loaned. At the end of the transaction, this flash loan is repaid by borrowing pairedLpTokens from Fraxlend.

However, the flash loan fee is not accounted for when borrowing from Fraxlend. The borrow amount from Fraxlend equals the flash loan amount unless users provide a greater overrideBorrowAmt.

The Natspec comments regarding this variable is: ”Override amount to borrow from the lending pair, only matters if max LTV is >50% on the lending pair”.

Since providing overrideBorrowAmt is not mandatory and there are no restrictions, the borrow amount from Fraxlend will usually be equal to _props.pairedLpDesired in most cases. However, this amount is equal to _d.amount, which is smaller than _flashPaybackAmt, causing the transaction to revert.

The minimum borrow amount from Fraxlend to repay the flash loan should be _props.pairedLpDesired + _d.fee.

Peapods Team: Resolved in the following code change.

FraxlendPair has a function pause which pauses all actions in the pair and a function pauseInterest which pauses interest accrual.

When interest is paused, new interest will not be accumulated and that's expected. However, once unpaused, the current borrowers will have to pay interest for the duration from the moment the protocol was paused until the current block.

Since the protocol may function normally and have just it interest paused, that means new lenders and borrowers may come and go. This will cause a huge interest misaccounting.

For example, interest is paused on Monday. Some borrowers leave the pair. and new ones enter it right before it's unpaused the next Monday. Since the lastUpdated timestamp will be the first monday, the new borrowers will immediately owe interest for that one week they were not even part of the pair.

Update the timestamp when pauseInterest(false) is called currentRateInfo.lastTimestamp = uint64(block.timestamp);

Peapods Team: Resolved

When calling deposit, a slippage of 0 and a deadline of block.timestamp is passed to _processRewardsToPodLp. Using 0 for slippage is dangerous as MEV bots could sandwich the swap to steal tokens.

Similarly, setting block.timestamp as deadline is ineffective as the transaction could sit in the mempool until it's ready to be processed, at which time block.timestamp is set, therefore offering no protection from sandwich attacks.

Allow user to input slippage and deadline parameters when depositing.

Peapods Team: Acknowledged.

Some EIP-4626 compliance issues have been observed in the AutoCompoundingPodLp contract: 1. According to EIP-4626, the mint function must mint exactly the user-inputted amount of shares, and the withdraw function must transfer exactly the specified assets amount. However, these functions mint or withdraw fewer tokens due to rounding down twice during the action flows.

During the withdraw function in the codebase:

• User provides _assets amount.
• It is converted to shares with convertToShares function, which rounds down.
• Then, the internal _withdraw function is called with this shares amount.
• In this internal function, the shares amount is converted to assets again with convertToAssets,

which also rounds down.

As a result, the actual assets amount transferred to the user is not the same as the user-provided amount. The same issue can be observed in the mint function as well. 1. According to EIP-4626, withdraw and redeem functions must support transaction flows where the msg.sender has an approval from the owner. However, in the codebase, withdrawals and redeems can only be performed by the owner, and approved users cannot execute these actions.

Update mint and withdraw functions to comply with the EIP specification and ensure the exact amounts are transferred. Also, update withdraw and redeem function to support approved users.

Peapods Team: Resolved in the following code change.

Owner can set a new protocol fee via setProtocolFee. However, because rewards are not updated prior to the fee change, the new fee will apply to previously accrued rewards.

For example, 100 PEAS in rewards were accrued since the last update. Fees are increased from 1 to 2%. An additional 1% of fees are unjustly applied to the accrued rewards.

In setProtocolFee, call _processRewardsToPodLp before setting protocolFee to the new fee.

Peapods Team: Resolved in the following code change.

When whitelisting a new vault with setVaultWhitelist, even if maxVault value has been reached, the new vault is still added to the whitelist due to using <= maxValue instead of "< maxValue" in the check.

Change from require(_vaultWhitelistAry.length <= maxVaults, 'M'); to require(_vaultWhitelistAry.length < maxVaults, 'M');

Peapods Team: Resolved in the following code change.

When a swap fails in depositFromPairedLPToken(), the amount that can be used in the next attempt is halved from the attempted swap amount.

The next time depositFromPairedLPToken() is called the fee will be based off the current balance of the contract, which will include the balance from the failed swap.

When _swapForRewards is called, it will reduce the _amountIn and _amountOut but still charge the admin fee based on the balance of the contract. This results in excessive admin fees paid over the multiple swaps.

Adjust the admin fee to match the actual amount that has been swapped.

Peapods Team: Resolved in the following code change.

getPriceUSD18 makes requests to a base and quote asset oracles. If any of them has been updated more than maxOracleDelay seconds ago, _isBadData will be set to true.

Since not all feeds have the same heartbeat, if the oracle uses two feeds with different ones, it may happen that one of the prices is stale, but it's accepted as a valid one.

Use two different delay variables - one for the base feed and one for the quote feed.

Peapods Team: Resolved in the following code change.

getPriceUSD18 returns _isBadData if the oracle price is stale. However, it doesn't consider the price going outside of the price range for the oracle's aggregator.

The price will be capped between minAnswer and maxAnswer of the aggregator. This will result in a wrong price being used in the protocol.

Even though most feeds have disabled their circuit breaker feature, there are still some that haven't, for example CVX/ETH

If the price goes outside the aggregator range, set _isBadData to true

Peapods Team: Resolved in the following code change.

Each time borrow or redeem is called in FraxlendPairCore, if there are insufficient local assets, then _depositFromVault is called to pull assets from the vault.

However, in _depositFromVault there is a check: if (depositLimit < _totalAsset.totalAmount(address(externalAssetVault))) revert ExceedsDepositLimit();

This check prevents the deposit from vault if the vault's allocated assets to the FraxlendPair exceeds the deposit limit.

Consider this example:

• FraxlendPair has a deposit limit of 10 ETH
• Vault has 30 ETH and allocates 50% to FraxlendPair
• Borrow/redeem actions cannot go through as the depositLimit check would always fail

Consider removing the depositLimit check from _depositFromVault. Instead, in LendingAssetVault, apart from percentage based allocations to a FraxLendPair vault, consider checking for the vault's depositLimit too to avoid over-allocation.

Peapods Team: Resolved in the following code change.

The price returned by the oracle is adjusted to 18 decimals with the following computation: _price18 = uint256(_price) * (10 ** 18 / 10 ** _decimals);

Notice that the division here happens before the multiplication. This means that if the decimals of the feed are more than 18, the price will be rounded to 0 causing big problems for the assets pricing.

Multiply price by 1e18 and divide afterwards.

Peapods Team: Resolved in the following code change.

When addLiquidityV2 is called, an amount of _pairedLPToken is transferred into the contract, and the same amount is used to add liquidity in the DEX_HANDLER.

However, if the pairedLPToken is a Fee-on-Transfer token, then the amount received would be less than expected due to a fee. Therefore, the DEX_HANDLER.addLiquidity call could fail due to insufficient tokens.

Use actual balance of pairedLPTokens when calling DEX_HANDLER.addLiquidity.

Peapods Team: Acknowledged.

When getting price from the oracle, if the base token is a pod, _getBaseTokenInClPool is called. There it gets all assets from the pod and assumes the first token in the array is the base token.

However, pods were designed to be multi-asset and able to be created permissionlessly. Therefore, if the first asset is not the intended base token, then serious integration issues would occur.

Furthermore in the function _debondFromSelfLendingPod, an assumption is made that the selfLendingPod has only one token. If ever this assumption is broken, there would be integration errors with FraxlendPair and a possibility of stuck tokens in LeverageManager after debonding.

In the constructor, similar to how UNDERLYING_TKN is defined, store the intended underlying token for the base (pod). Furthermore, consider handling the case where a SelfLendingPod has multiple tokens.

Peapods Team: Acknowledged.

When the base token is one of the two tokens in the UNDERLYING_TKN_CL_POOL, it has to be token1. That's because _pricePTKNPerBase18 is calculated based on whether or not the base token is part of that pool by checking if it's token1.

This means for pools where the base token is token0 the price will be wrongly flipped.

If the base token is included in the pool pair, always make sure it's the token1.

Peapods Team: Resolved.

Similarly to M-01, FraxlendPairCore::repayAssetWithCollateral() & FraxlendPairCore::leveragedPosition() does not allow a user to set the block.timestamp for their swap.

This exposes users to MEV sandwich attacks, and can cause them to lose out on funds that would have been used to repay their debt.

Allow users to input the deadline for the swaps.

Peapods Team: Resolved in the following code change.

The FraxlendPair contracts do not support non-standard tokens such as rebasing or fee-on-transfer tokens, whose balance changes during transfers or over time. If the Peapods team expects to support these tokens, then there will be accounting issues when interacting with FraxlendPair contracts.

Verify the amount of tokens transferred to the contracts before and after the actual transfer to infer any fees/interest.

Peapods Team: Acknowledged.

This bug was reported in a previous audit but does not seem to be fixed (see

https://hackmd.io/@tapir/SyxqzohUA#M-6-Removing-leverage-will-likely-fail-if-the-pod-token-needs-

to-be-sold-for-the-borrowed-token-in-a-self-lending-scenario)

The issue remains that there is no natural Uniswap V2 market that exists to swap pod tokens for borrowed assets (from FraxlendPair), when the pair is self-lending.

Furthermore, during _swapPodForBorrowToken in the removeLeverage flow, the entire amount of pod tokens received is passed as amountInMax for the swap --resulting in zero slippage protection.

This could allow an attacker to deploy a pool to take advantage of this scenario and steal all pod tokens from a user.

Implement proper slippage protection for the swap, and ensure that a healthy Uniswap V2 market exists for the swapping of self-lending pairs.

Peapods Team: Resolved in the following code change.

When the staking pool token is transferred it will call _setShares(), which will lead to _distributeReward() being called.

Inside of _distributeReward(), it will loop through the reward tokens and transfer any rewards to the users. If a user becomes blacklisted from using USDC, _distributeReward() will revert.

This, in turn, will lead to the tokens being stuck in the users wallet and become untransferable. Additionally, this prevents a user from calling claimRewards(). They will not be able to claim any other rewards tokens earned outside of USDC.

Instead of pushing rewards to users automatically, rely on them claiming the rewards themselves and allow them to specify which token they would like to claim.

Peapods Team: Resolved in the following code change.

Multiple contracts in the system -V3TwapUtilities, V3AerodromeUtilities, UniswapV3SinglePriceOracle - fetch the TWAP price of a given asset. When calculating the TWAP, the delta of two CL ticks is divided by a given time period.

Since solidity truncates when it divides, for negative ticks the result will be rounded up instead of rounded down resulting in a different price. For reference, see how is this handled in OracleLibrary.

Implement the same solution as in OracleLibrary

Peapods Team: Resolved in the following code change.

The maximum a user can borrow is determined by the exchange rate returned by the oracle. However, all oracles are susceptible to front-running as their prices tend to lag behind an assets real price.

For example, Chainlink oracles are updated after price crosses a threshold while Uniswap V3 TWAP returns price over past X blocks. An attacker could exploit the difference between the price reported by an oracle and the asset's actual price to gain a profit by front-running the oracle's price update.

The likelihood of this condition is increased for Peapods due to the multiple layers that an underlying asset is wrapped in.

Consider this example:

• Fraxlend Vault has a high maxLTV of 95%, with the collateral as aspTKN (pPEAS-WETH) and asset

(WETH)

• 1 aspTKN is currently worth 0.002 WETH
• Price of aspTKN drops while WETH price increases, such that 1 aspTKN should be worth 0.0015

WETH

• Due to the lag in oracle update, price is not updated yet
• Attacker sees this opportunity and front-runs the oracle update to:
• Deposit 100 aspTKNs
• Max borrow 0.19 WETH (95% LTV)
• Afterwards, the oracle price is updated to 1 aspTKN = 0.0015 WETH
• The attacker's position is now unhealthy as his collateral is worth less than the loan amount
• Attacker back-runs the oracle update to liquidate himself:
• To seize 100 aspTKN he repay 0.15 WETH
• Gains back his original collateral plus 0.04 WETH

All profits gained result in bad debt socialized among lenders.

Consider adding a borrowing fee to mitigate arbitrage opportunities.

Peapods Team: Acknowledged.

The mint function in the FraxlendPairCore contract rounds down in favour of the users and users pay fewer assets for corresponding shares.

Round-up in favour of the protocol.

Peapods Team: Resolved in the following code change.

The current logic in getPrices will underflow when the BASE token has more than 18 decimals: uint256 _priceOne18 = _priceBaseSpTKN * 10 ** (18 - IERC20Metadata(BASE_TOKEN).decimals());

This will entirely prevent oracle compatibility with borrow tokens that have more than 18 decimal precision, which is problematic in Peapods which is a permissionless system.

Query the decimals first and if it is greater than 18, subtract 18 from the base token’s decimals.

Peapods Team: Resolved in the following code change.

Fee on transfer tokens are not supported correctly for:

• pod underlying token - bond() doesn't check the received amount of the transfer and mints shares

based on the initial amount

• pod paired token - LeverageManager assumes it has received the whole amount of the flashloaned

token

Consider supporting fee on transfer tokens

Peapods Team: Acknowledged.

Chainlink recommends that all Optimistic L2 oracles consult the Sequencer Uptime Feed to ensure that the sequencer is live before trusting the data returned by the oracle.

See https://docs.chain.link/data-feeds#l2-sequencer-uptime-feeds

If the Arbitrum sequencer goes down for example, oracle data will not be updated and could become stale. Attackers could take advantage of the stale prices and carry out attacks, such as borrowing more against their collateral's true value.

Follow the code example of Chainlink:

https://docs.chain.link/data-feeds/l2-sequencer-feeds#example-code

Peapods Team: Resolved in the following code change.

When AutoCompoundingPodLp swaps the paired tokens for pod tokens, it adds them as liquidity. However, if the amounts are too small, this will result in minting 0 liquidity and a revert with INSUFFICIENT_LIQUIDITY_MINTED.

Just like in the DecentralizedIndex, consider rewards only if they exceed a given minimum.

Peapods Team: Resolved in the following code change.

Owner can set the lockupPeriod variable via setLockupPeriod. However, this would affect all users who are already staked with the previous lockupPeriod value, which would be unfair.

When a user stakes, consider storing the current lockupPeriod value in their own Stake struct. And use that value when checking for unlock time in unstake.

Peapods Team: Resolved in the following code change.

The idea of AutoCompoundingPodLp is to swap the accumulated rewards into paired lp tokens and use them to generate new spTokens.

When the current reward token of the asp doesn't match the reward token for its pod, 0 slippage is used for the swap so anyone can sandwich the transaction to benefit from it which will result in a loss for the asp holders.

The following can be executed in one transaction:

• swap
• process rewards
• swap again

Consider adding an adequate slippage parameter to the swaps and a try/catch as well to not introduce a new way of DOS-ing the asp.

Peapods Team: Resolved.

In AutoCompoundingPodLp::withdraw(), it will convert the amount of assets to shares prior to calling _processRewardsToLp(). Then it will convert the shares back to assets.

In between conversions, the _cbr() is likely to increase due to the increase in total assets that will occur when _processRewardsToLp() is called. This will lead to a larger output amount of assets than requested to be withdrawn.

When AutoCompoundingPodLp::mint() is called, it will convert the amount of shares to assets. Then call _processRewardsToLp(), and proceed to convert the amount of assets back to shares.

This will have the inverse effect, and provide a smaller amount of shares for the deposited user than requested. Ultimately, both of these functions will provide users with different amount of shares and assets respectively than expected.

_processRewardsToLp() should be called in the beginning before any conversions.

Peapods Team: Resolved.

Guardian Team: In functions withdraw and redeem asset-share calculations are made with a stale cbr since conversions are made before calling _processRewardsToLp(). This will lead to incorrect outputs for the user.

Similarly to M-20, the LendingAssetVault::withdraw() and LendingAssetVault::mint() perform asset and share conversions with an update to _cbr() taking place in between.

This takes place with the call to _updateInterestAndMdInAllVaults() happening in _withdraw() and _deposit(). This will lead to a similar scenario where the accounting for assets will be incorrect for withdrawals and the shares will be incorrect for mints compared to the amounts requested.

_updateInterestAndMdInAllVaults() should be called in the beginning rather than between conversions.

Peapods Team: Resolved in the following code change.

Guardian Team: LendingAssetVault mint performs convertToAssets before updating for interest. This will lead to users minting not getting the amount of shares requested, which is against ERC4626 spec and unexpected for users.

Currently, all pods are priced equally in the VotingPool contract (assuming the conversion factor is 1:1). This means users can stake cheap pods and receive the same amount of voting tokens they would have received with more expensive pods.

For example, a pod with DAI as underlying token and a pod with WETH as underlying token would both be priced equally if their conversion factors are the same.

There may also be a situation where the pod with DAI token has its conversion factor higher - this will lead to the DAI pod minting more voting tokens than the WETH one.

Either implement another pricing mechanism or make sure to only use pods with close price.

Peapods Team: Acknowledged.

If the underlying token of a pod is a Fee-on-Transfer token, the accounting when staking in VotingPool would be inaccurate. The balance of tokens after fees should be accounted for instead. Multi-asset pods are also incompatible as _calculateCbrWithDen assumes _asset[0] is the only token in the pod.

As Peapods is expected to be permissionless and work with all types of tokens, handle such non-standard tokens accordingly.

Peapods Team: Acknowledged.

When LAV.redeemFromVault() is called, FraxlendPair.redeem() is called and the returned value (the assets received) are subtracted from the vaultUtilization.

Since vaultUtilization is adjusted by dividing in _updateAssetMetadataFromVault, vaultUtilization may end up being 1 wei less than the received assets. Because of this the redeemFromVault transaction will fail.

Subtract the minimum between the received assets and vaultUtilization.

Peapods Team: Resolved in the following code change.

During liquidation in FraxlendPair, if bad debt was incurred, whitelistUpdate is called which updates of all whitelisted vaults (except the calling vault).

Then the bad debt is realized via: totalAsset.amount -= _amountToAdjust before LendingAssetVault is updated again to reduce its own internal tracking for _totalAssets.

Instead, the whitelistUpdate of all vaults should be performed after the bad debt is realized in FraxlendPair. As a result, all other vaults will assume a higher amount of available assets (did not account for lost assets from bad debt) and charge a higher interest.

Call whitelistUpdate at the end of liquidate after all state changes have been made in FraxlendPair

Peapods Team: Resolved in the following code change.

spTKNMinimalOracle uses the same twapInterval for two different pools. Depending on the available liquidity on the two pools and the assets volatility, one twap period may not be sufficient to get accurate prices for both pools.

Consider having a different interval for each pool.

Peapods Team: Acknowledged.

LeverageManager._getPairedTknAndAmt uses 0 as slippage parameter when bonding tokens. In result, the received pToken amount may be too small that it causes significant loss for the user.

Allow the user to input their slippage.

Peapods Team: Acknowledged.

FraxlendPair.getPairAccounting includes the unlent assets from the LAV. External integrations that depend on that function will receive wrong information.

For example, if they calculate the value of a single share using the output of that function, their result will be wrong because they account for assets not present in the pair.

Exclude the LAV assets.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

Function donate aims to increase the totalAssets of the LendingAssetVault without increasing the totalSupply of shares, hence a donation.

The issue is that _burn(address(this), convertToShares(_assetAmt)); converts the _assetAmt to shares after _deposit(_assetAmt, address(this)); already minted shares, so the newly calculated share amount to burn will be less than the calculated and minted shares in _deposit.

Ultimately, function donate increases the totalSupply even though it is not meant to.

Burn the entire added supply post-deposit.

Peapods Team: Resolved in the following code change.

When a deposit or withdraw happens in the LendingAssetVault all pairs should be updated because the totalAssets are increased unilaterally which affects the pairs.

There is a function which allows setting _updateInterestOnVaults to be set to false. If so, any updates to all pair at once will be skipped. This means users can manipulate pairs' utilization rates by depositing/withdrawing.

_updateInterestOnVaults is meant to be set to false if updating all vaults start causing OOG errors. Given the problem it creates and the facts that there is a limit to the maximum pairs that can be connected to a vault and that pairs can also be removed, the removal of _updateInterestOnVault is best.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

Pods have a property hasTransferTax which when enabled, a fee-on-transfer is taken from the value to be transferred and the recipient receives less tokens.

This is a problem for the LeverageManager.addLeverage() function because it assumes the whole amount has been received and assigns that amount to LeverageFlashProps.podAmount.

Later, when the podAmount is requested from the IndexUtils, the transaction will revert because the LeverageManager contract doesn't have all the tokens.

Consider the fee on transfer aspect of the pod tokens when adding leverage.

Peapods Team: Acknowledged.

Proof of concept: PoC

In _processRewardsToPodLp, if a swap of the main reward token fails, an override feature kicks in to halve and store the next _amountIn to swap in _tokenToPairedSwapAmountInOverride.

A temporary DOS attack can be carried out as such: 1. Donate 50 wei of the main reward token (PEAS), assuming contract has no previous balance of PEAS. 2. Call deposit to trigger _processRewardsToPodLp where the small swap to Uniswap V3 would fail due to insufficient amountOut. 3. Half of 50 wei (i.e. 25 wei) will then be stored in the override mapping. 4. Contract will attempt to swap for another 6 times before the override amount is set to zero — preventing actual rewards from being processed.

Consider re-designing the error handling for the swap.

Peapods Team: Resolved in the following code change.

Proof of concept: PoC

In depositFromPairedLpToken, if a swap of a reward token fails, an override feature kicks in to halve and store the next _amountIn to swap in _rewardsSwapAmountInOverride.

A temporary DOS attack is possible by making use of this feature: 1. Deposit 50 wei of the reward token. The small swap to Uniswap V3 would fail due to insufficient amountOut. 2. Half of 50 wei (i.e. 25 wei) will then be stored in _rewardsSwapAmountInOverride. 3. Contract will attempt to swap for another 6 times before the override amount is set to zero — preventing actual rewards from being processed.

Consider redesigning the override design for swap failures.

Peapods Team: Resolved in the following code change.

Numerous addresses are set as constant variables in the protocol. However, a multitude of these addresses are specific to Ethereum Mainnet and are either not deployed or occupied by an EOA on other chains, such as Arbitrum and Base.

This will lead to reverts when they are interacted with when calling addLeverage(), and make the functionality unusable outside of Ethereum Mainnet.

Here is a list of variables that are assigned a constant address that is either incorrect or not deployed outside of Mainnet:

• DAI
• PROTOCOL_FEE_ROUTER
• REWARDS_WHITELIST
• STYETH
• YETH,
• WETH_YETH_POOL,
• V3_ROUTER

Use an immutable instead of a constant for the addresses, and pass in the proper addresses in the constructor on deployment.

Peapods Team: The issue was resolved in commit 9c1d3c2.

During getPrices, in order to correctly price each pTKN _accountForCBRInPrice is called internally. There it first checks unlocked = 1 which is the reentrancy guard in the pod contract. However, in older versions of the pod contract which are currently live, unlocked is not a uint but a boolean.

Therefore, this call to older pod contracts will always revert and fail, making the oracle incompatible with pods such as pPEAS and pOHM which hold the bulk of the protocol's TVL (see

https://etherscan.io/token/0x027CE48B9b346728557e8D420Fe936A72BF9b1C7?a=0x80e9c48ec4

1af7a0ed6cf4f3ac979f3538021608#code).

Ensure that the oracle is compatible with the interfaces of older pod contracts.

Peapods Team: Acknowledged.

Proof of concept: PoC

Each underlying asset of the POD token has a weight assigned to it which determines how much of that token should be paid. If the weights for tokens A and B are 50 and 100, this means for each token A, 2 token B should be paid.

The WeightedIndex contract computes the _tokenAmtSupplyRatioX96 variable by dividing the amount of underlying tokens the user is paying by the total amount of tokens held in the contract.

This variable is used in two places:

• to determine how much Pods will the user receive
• to calculate the amount of the other underlying tokens that the user must pay.

The problem is that balanceOf can be manipulated by anyone by sending tokens to the contract. Let's take a look at the following example:

• Two tokens - A and B - with weights 50 and 100.
• Alice wraps 2A + 4B = 2 Pod
• A third party sends 2A to the contract. Total A = 4
• Bob comes and tries to wrap 2A + 4B.
• Since the ratio is 2A / 4A = 1/2, he receives 1 Pod and pays 2A + 2B.

We can see how the ratio A:B changed from 1:2 to 1:1 which diverts from the expected ratio of the pod and the backing ratio users expect when entering a pod.

Consider tracking the underlying token balances in an internal mapping.

Peapods Team: Resolved in the following code change.

In the paymentAmount function, FLASH_FEE_AMOUNT_DAI is called on the pod contract to obtain the flash fee.

However, for existing pod contracts which are live (e.g. pPEAS) the flash fee is named FLASH_FEE instead. So calls to these pods will always fail. LeverageManager.addLeverage will therefore also fail if the flash source is a pod.

Ensure that PodFlashSource is compatible with both old and new pod contracts.

Peapods Team: Acknowledged.

If a malicious pod were to be used in LeverageManager, it would allow reentrancy in the add/remove leverage functions. The attacker would be able to access the critical callback function and provide arbitrary data to steal other users' funds.

This is currently prevented by owner-approved flashSource and lendingPairs. However, if a malicious pod were to be accidentally approved, the consequences would be severe.

Be extra careful about the approvals for flashSource and lendingPairs. Consider validating that the caller in callback is a whitelisted flash source.

Peapods Team: Acknowledged.

According to EIP-4626, maxMint should return 2**256 - 1 if there is no mint limit. The current implementation of the function returns type(uint256).max - 1 which is equivalent to 2**256 - 2.

Return type(uint256).max.

Peapods Team: Resolved in the following code change.

debond() checks if a user is withdrawing 98% or more of the total supply. If they are, then they do not have to pay a fee when debonding. A malicious user could take out a flashloan and bond to increase their share of the total supply to reach the target 98%, then debond right away to avoid paying fees.

Charge the fee to users unless they are debonding 100% of the total supply.

Peapods Team: Resolved in the following code change.

clBaseFeed is assigned to CHAINLINK_BASE_PRICE_FEED in the constructor. Later, in the Chainlink oracle when the QUOTE/BASE price is required, if this variable is not set the result will be QUOTE/USD.

Since there is no validation in the constructor, it's possible to not set the clBaseFeed. This will be okay for feeds where the BASE asset is USD, but otherwise the pricing will be incorrect.

The best solution is to add validation in the constructor.

Peapods Team: Resolved.

When a flashloan is taken from PodFlashSource.sol, it calls DecntralizedIndex::flash(). The fee for the flashloan is taken in DAI and transferred to the RewardsToken.sol if DAI is the PAIRED_LP_TOKEN and not the reward token.

The DAI will remain dormant in the contract since it is not added to the rewardsPerToken mapping, and stakers will not receive the proper rewards during that time period.

Call depositFromPairedLPToken() after the fee is taken when DAI is the PAIRED_LP_TOKEN and not the reward token.

Peapods Team: Resolved in the following code change.

The _assetDecimals function in LendingAssetVault is not needed.

Consider removing the function.

Peapods Team: Resolved.

During the liquidation of a borrower in a FraxlendPair vault, any debt that cannot be repaid (i.e. bad debt) is socialized among all lenders to the vault which includes the LendingAssetVault (LAV).

Within the LAV, the bad debt from one vault is further socialized among depositers to the LAV. The issue lies with lenders/depositors who can avoid the socialization of bad debt by front-running a liquidate call, therefore putting a greater burden on the other lenders.

Clearly document this risk to users.

Peapods Team: Acknowledged.

When a pair is removed by calling setVaultWhitelist(vault, false), the vaultWhitelistAryIdx and vaultMaxPerc variables for that pair are not cleared.

Clear these variables.

Peapods Team: Resolved in the following code change.

The function whitelistUpdate can either update one specific vault or all vaults depending on the boolean passed in. When the boolean is false, all vaults but the calling vault is updated since msg.sender is passed as the _vaultToExclude.

It is unclear if this is the intended behavior as the natspec comments indicate that all vaults should be updated. However, if the calling vault was included in the update, addInterest may revert due to reentrancy protection in the FraxlendPair contract.

Be aware that not all vaults are updated when whitelistUpdate(false) is called, and consider updating the natspec comments for accuracy.

Peapods Team: Acknowledged.

Leverage can be added to positions by the owner of the position NFT or any approved party of that NFT. However, the opposite action - removing leverage - can be performed only by the owner of the NFT.

Document this behavior.

Peapods Team: Resolved in the following code change.

When a stale price is passed from the oracle, it will return the price as 0 and emit an alert in _updateExchangeRate(). If the highExchangeRate is 0, then it will end up reverting in the calculation of _deviation because it divides by 0.

This will lead to the event never being emitted and transactions will fail without a clear root cause.

Instead of emitting a log when there is bad price data, revert with a custom error to signify that the price is wrong.

Peapods Team: Acknowledged.

When a stale price is passed from the oracle, it will return the price as 0 and emit an alert in _updateExchangeRate(). If the highExchangeRate is 0, then it will end up reverting in the calculation of _deviation because it divides by 0.

This will lead to the event never being emitted and transactions will fail without a clear root cause.

Instead of emitting a log when there is bad price data, revert with a custom error to signify that the price is wrong.

Peapods Team: Acknowledged.

_accountForCBRInPrice() validates that the pod is not currently locked, and otherwise reverts. When a user takes out a flashloan from a pod it locks the contract, and will prevent getPrices() from being called.

This will prevent borrowAsset() (and any other function that calls updateExchangeRate()) from executing from the pod that has taken the flashloan.

Without the lock validation, the oracle price would be manipulatable via a flashloan. Document that using PodFlashSource can lead to a revert from locking the pod.

Peapods Team: Acknowledged.

When users stake, the amount of voting tokens they get is determined by multiplying the deposited amount by the factor. In unstake the opposite is done - the unstaked amount is divided by the factor. This will result in users receiving less tokens than they initially deposited.

Document the discrepancy

Peapods Team: Acknowledged.

AutoCompoundingPodLp guards against the classic inflation attack by internally tracking deposits with the _totalAssets variable. However, the attack is still possible by sending reward tokens which are converted into assets before each action (e.g. deposit/withdraw). This increases _totalAssets without minting new shares.

If the AutoCompoundingPodLp is created by the factory contract, a minimum deposit is performed with 1e3 assets that creates 1e3 shares. This mitigates the effect of a donation but an attack is still possible.

Consider this example:

• After factory creation with min deposit: _totalAssets = 1e3, totalSupply = 1e3
• Attacker donates the equivalent of 1000e18 assets
• User deposits 1e18 assets:
• _cbr: (1000e18 + 1e13) * 1e18 / 1e3 -> 1.00..03e36 (v large number)
• convertToShares: 1e18 * 1e18 / 1.00..03e36 -> round down to 0
• User receives 0 shares and loses assets

The larger the donation, the higher the threshold will be for user's deposits to round down to 0 shares.

Consider performing a larger minimum deposit by the factory contract, which then increases the cost of the attack. Also, during deposit, after convertToShares is performed, checked that shares are not equal to 0 or else revert.

Peapods Team: Resolved in the following code change.

Removing leverage can only be done by position NFT owners, but adding leverage can be done by any approved user or operator. When adding leverage, the msg.sender provides their own funds as pod tokens, and pod token refunds are returned to the msg.sender as expected.

However, flash-loaned pairedLpTokens are also refunded to the msg.sender, not necessarily to the position owner. When the position owner has an over-collateralized position in Fraxlend, an approved user can flash loan more pairedLpToken than needed and receive the entire refund.

Reconsider who should receive the flash loan refunds. If the msg.sender is expected to receive them, rather than the position owner, consider documenting this behavior.

Peapods Team: Resolved in the following code change.

Inside of getFullUtilizationInterest(), the _newFullUtilizationInterest will always go down when the current utilization is less than MIN_TARGET_UTIL.

It does not take into consideration if the Fraxlend vault’s utilization has actually gone up or down, and simply looks at if the utilization is within certain target ranges.

Be aware that this will occur, and that it will only go down based on your configuration of MIN_TARGET_UTIL.

Peapods Team: Acknowledged.

_exchangeRateInfo is cached to memory in the isSolvent modifier of the FraxlendPairCore contract. However, this cached value is never used but the storage variable is being used instead.

Consider using the cached memory variable. Alternatively, remove the variable.

Peapods Team: Resolved in the following code change.

In _swapNativeForTokensWeightedV2 , a loop is performed to swap WETH for each index token.

However by calling swapExactTokensForTokensSupportingFeeOnTransferTokens, all WETH (tokenIn) is swapped for tokenOut on the first loop iteration, leaving no remaining WETH for next iteration, if there are multiple assets in an index.

Then, there will be insufficient index tokens to bond when _indexFund.bond is later called.

Use the other Uniswap function swapTokensForExactTokens instead to get out a specific amount of each index token. Alternatively, calculate getAmountsIn to pass in exactly the amount of WETH needed before calling the swap function.

Peapods Team: Acknowledged.

If a POD is transferred from a V2 pair, a buy fee is applied. If it's transferred to a V2 pair, a sell fee is applied. This means that if a POD is transferred from a V2 pair to the pair itself, both fees will be applied.

On top of that only the second fee is deducted from the amount to be subtracted. Imagine the following:

• Pair tries to transfer 20 tokens
• Buy fee is 50% so it pays 10 tokens
• Sell fee is 10% so it pays 2 tokens
• Since sell fee was the last recorded fee, the amount transferred to the recipient is (20 - 2) = 18
• Total transferred: 30 tokens

Instead of if-if, change the fee checks to if-else if

Peapods Team: Resolved in the following code change.

In bondWeightedFromNative, when _stakeAsWell is true, only half of msg.value is passed to _swapNativeForTokensWeightedV2. However, later during _swapForIdxToken the entire contract's balance of ETH is converted to WETH.

This results in a revert later on when _zapIndexTokensAndNative is called as _zap attempts to convert the remaining half of msg.value to WETH:

a Guardian proof of concept

Consequently, this protocol functionality becomes unusable.

If _stakeAsWell is true, only half of msg.value should be converted to WETH initially.

Peapods Team: Resolved in the following code change.

Flash loans from UniswapV3 pools are taken for the paired token of the leveraged pod. If the second token in that pool matches with the reward token for the ASP, the swap will fail because swapping will be locked since flashloan is taken from that pool.

Make sure the Peapods team is aware of this

Peapods Team: Acknowledged.

When leverage is being removed, the asp collateral has to be turned to spTokens via calling redeem. This action will trigger rewards distribution and will swap pairedTokens for pod tokens.

The price of the pairedTokens will drop while the price of the pod tokens will go up. When the spToken is unwrapped to LP token and liquidity is removed, because of the previous swap the user will receive more paired tokens and less pod tokens.

This should be resolved with implementing the one-sided liquidity formula.

Peapods Team: Resolved in the following code change.

When an fToken is bonded for a self lending pod, the bond function will transfer slightly less fTokens from the LeverageManager because of rounding. This will leave the LeverageManager with non-zero approval that will increase over time.

Also, if all of the paired assets are used up, the refund if statement won't be executed and the msg.sender won't receive these fTokens. They will be left in the contract and the next caller will have access to them.

Document this behavior

Peapods Team: Acknowledged.

remvoe instead of remove is within the NatSpec.

Fix the typos

Peapods Team: Resolved.

LinearInterestRate cannot be used because it implements the IRateCalculator interface where the getNewRate signature is getNewRate(bytes,bytes). FraxlendPairCore uses IRateCalculatorV2 with signature getNewRate(uint256,uint256,uint64).

Change the LinearInterestRate to adhere to the IRateCalculatorV2 interface.

Peapods Team: Acknowledged.

In addLeverage a user can intentionally borrow up to the solvency limit from Fraxlend by passing in desired _overrideBorrowAmt.

The caller of addLeverage will then receive any additional borrow tokens while putting the leveraged position on the edge of liquidation. This opens up an attack surface where borrowed funds leave the system.

This could also be abused by an approved account or if the user accidentally sets isApprovedForAll to his positionNFT.

Unless there are strong reasons to do so, do not allow a user to override borrow amount. Else, consider restricting the override borrow amount to provide some additional buffer from liquidation.

Peapods Team: Acknowledged.