Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

validateVariablePoolHasEnoughLiquidity will always fail causing complete DOS to the protocol #244

Closed
howlbot-integration bot opened this issue Jul 8, 2024 · 2 comments
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-218 🤖_10_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality

Comments

@howlbot-integration
Copy link

Lines of code

https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/CapsLibrary.sol#L68

Vulnerability details

Impact

buyCreditMarket, sellCreditMarket and liquidateWithReplacement all call validateVariablePoolHasEnoughLiquidity in the end of their execution to make sure that there is enough liquidity in the AAVE V3 pool. The issue is that the validation in validateVariablePoolHasEnoughLiquidity is critically wrong and will fail when used with the actual AAVE protocol.

As a result borrowing and lending will be completely blocked.

Proof of Concept

Here is the validation performed in validateVariablePoolHasEnoughLiquidity:

uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
if (liquidity < amount) {
    revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
}

As we can see, the balance of the underlying borrow token (USDC) of the pool is derived as the balance of address(state.data.variablePool). The issue is that AAVE stores the underlying tokens in the individual AToken smart contracts, not the Pool contract. There are multiple ways we can understand this.
Firstly, looking into the USDC balances of the Pool and AToken contracts on Etherscan.
Secondly, looking into the supply and withdraw functions on AAVE:
1/ Pool.sol -> supply() -> SupplyLogic.executeSupply():

IERC20(params.asset).safeTransferFrom(msg.sender, reserveCache.aTokenAddress, params.amount);

We can see in the executeSupply function, called in supply, that the supplied underlying tokens are transferred directly from the sender (Size) to the AToken contract.

2/ Pool.sol -> withdraw() -> SupplyLogic.executeWithdraw():
Here we can see that no token transfers are occuring and the transfers are actually performed in the AToken contract (https://etherscan.io/address/0x98C23E9d8f34FEFb1B7BD6a91B7FF122F4e16F5c#readProxyContract), in the burn function:

function burn(
  address from,
  address receiverOfUnderlying,
  uint256 amount,
  uint256 index
) external virtual override onlyPool {
  _burnScaled(from, receiverOfUnderlying, amount, index);
  if (receiverOfUnderlying != address(this)) {
    IERC20(_underlyingAsset).safeTransfer(receiverOfUnderlying, amount);
  }
}

As a result, buyCreditMarket, sellCreditMarket will always fail when calling
validateVariablePoolHasEnoughLiquidity, and lending/borrowing will never work.

Tools Used

Manual review

Recommended Mitigation Steps

In validateVariablePoolHasEnoughLiquidity, use the AToken contract, not the Pool contract:

IAToken aToken = IAToken(state.data.variablePool.getReserveData(address(state.data.underlyingBorrowToken)).aTokenAddress);
uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(aToken));

Assessed type

Invalid Validation

@howlbot-integration howlbot-integration bot added 3 (High Risk) Assets can be stolen/lost/compromised directly 🤖_10_group AI based duplicate group recommendation bug Something isn't working duplicate-218 sufficient quality report This report is of sufficient quality labels Jul 8, 2024
howlbot-integration bot added a commit that referenced this issue Jul 8, 2024
@c4-judge c4-judge added the satisfactory satisfies C4 submission criteria; eligible for awards label Jul 11, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge c4-judge added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value downgraded by judge Judge downgraded the risk level of this issue and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Jul 21, 2024
@c4-judge
Copy link
Contributor

hansfriese changed the severity to 2 (Med Risk)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working downgraded by judge Judge downgraded the risk level of this issue duplicate-218 🤖_10_group AI based duplicate group recommendation satisfactory satisfies C4 submission criteria; eligible for awards sufficient quality report This report is of sufficient quality
Projects
None yet
Development

No branches or pull requests

1 participant