code-423n4 / 2024-06-size-validation

1 stars 0 forks source link

Size can overestimate liquidity on borrowing and lending actions after Aave upgrades to v3.1 #197

Closed c4-bot-10 closed 3 months ago

c4-bot-10 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-06-size/blob/main/src/libraries/CapsLibrary.sol#L67-L71

Vulnerability details

Bug Description/Proof of Concept

Aave is planning their v3.1 upgrade, which includes virtual accounting. As described in these two sources, Aave will no longer query the balanceOf the AToken in order acquire the available liquidity. This previous method includes capturing any "excess" tokens that were sent ("donated") to the AToken contracts. However, the new virtual accounting feature will now only track liquidity that flows through the pool contracts, i.e. "donated" tokens in the AToken contracts will no longer be considered part of the available liquidity. Therefore, the balance of the AToken contract can be greater than the virtual accounting balance, but only the virtual accounting balance will be considered during actions (such as supply and withdraw).

Size implements liquidity checks for borrow and lending actions and reverts if there is not enough liquidity available on Aave for the borrower to retrieve their borrowed funds:

Size::buyCreditMarket

178:    function buyCreditMarket(BuyCreditMarketParams calldata params) external payable override(ISize) whenNotPaused {
...
184:        state.validateVariablePoolHasEnoughLiquidity(amount); // @audit: liquidity check

Size::sellCreditMarket

188:    function sellCreditMarket(SellCreditMarketParams memory params) external payable override(ISize) whenNotPaused {
...
194:        state.validateVariablePoolHasEnoughLiquidity(amount); // @audit: liquidity check

CapsLibrary::validateVariablePoolHasEnoughLiquidity

67:    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
68:        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool)); // @audit: Aave will use virtual accounting for 3.1 upgrade
69:        if (liquidity < amount) {
70:            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
71:        }

As we can see above, Size uses balanceOf to query the available liquidity, which works fine for the current version of Aave V3 pools. However, once Aave upgrades to v3.1 the balanceOf can return a value that is greater than the actual tracked virtual accounting balance (actual liquidity). The above function is therefore not future proof and is likely to overestimate liquidity after Aave upgrades.

Impact

Liquidity check on borrow and lend actions can overestimate available liquidity, allowing these actions to succeed even when there may not be enough liquidity available for the borrower. This can then result in the borrower being unable to withdraw all of their borrowed assets from Size.

Tools Used

manual

Recommended Mitigation

Virtual accounting is a major feature of Aave v3.1 that will affect all "normal" reserves. Therefore, Size should include logic that will handle this new feature in order to integrate with Aave in a secure, future-proof manner. Depending on when Size deploys to mainnet, different options are available in order to mitigate this issue:

Assessed type

Other

0xJCN commented 3 months ago

Hi @hansfriese,

This issue discusses the same underlying root cause as issue 368 and therefore I believe this issue should be satisfactory and a duplicate of 368. Thank you.

hansfriese commented 3 months ago

I have left a comment on #368.

0xJCN commented 3 months ago

Thank you I saw the comment. Does that mean that this issue will also be moved to the findings repo and labeled as a satisfactory QA?

hansfriese commented 3 months ago

Please let me know your QA report in the findings repository. If your QA report has received a grade A or grade B, I will add 1 point for this instead of moving.

0xJCN commented 3 months ago

I did not submit an official QA report for this contest

hansfriese commented 3 months ago

I got it. We don't need to move this finding then.

0xJCN commented 3 months ago

Understood, thank you