Users might not be able to withdraw USDC from the protocol

[howlbot-integration]

Lines of code

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

Vulnerability details

Impact

Users might not be able to withdraw USDC from the protocol

Proof of Concept

After some operations, there is this check such as taking a loan, there is this check:

function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));
        if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }
    }

It has the goal to not allow taking out loans whenever there would not be enough liquidity in Aave for the user to convert the internally used version of USDC into actual USDC by withdrawing. The issue is that the liquidity is computed by using balanceOf which can lead to incorrect data.

Whenever a user deposits into the protocol, supply() is called on the Aave pool which mints a special token to the receiver showing the amount he deposited and thus, what he can withdraw. Then, upon withdrawing, withdraw() is called which burns those tokens. If a malicious user directly sends USDC into the Aave pool, then balanceOf would increase but no new tokens will be minted.

Imagine the following scenario:

1. There are 90 tokens in the pool
2. After a borrow, validateVariablePoolHasEnoughLiquidity() is called with an amount of 100
3. This should revert but a malicious user frontruns the borrow and directly sends 10 tokens into the pool
4. Now, the function passes but the actual aToken balance of the pool is still 90 as those 10 were directly sent
5. User tries to withdraw the 100 tokens from the protocol but it fails as it tries to burn 100 tokens when only 90 are actually available

Tools Used

Manual Review

Recommended Mitigation Steps

Avoid using a manipulatable value like balanceOf

Assessed type

Other

[c4-judge]

hansfriese changed the severity to 2 (Med Risk)

[c4-judge]

hansfriese marked the issue as duplicate of #152

[c4-judge]

hansfriese marked the issue as satisfactory

[samuraii77]

Hi, this issue should not be duplicated with the one it is currently duplicated to. Take a look at #225 which is another issue I have reported, they are duplicated to each other. 225 is properly duplicated however this one is not. These 2 issues have 2 different root causes and should not be a duplicate of each other.

[gesha17]

Hi @samuraii77,

Indeed it seems this one should not be duplicate of #152.

In the scenario you describe, attempting to create a loan with 100 borrowed tokens, when only 90 are available in Size's account would fail. The call to transfer borrowAToken from the lender to the msg.sender(borrower) would fail, since there would be only at most 90 borrowATokens available in the lender's account.

Essentially, you can't create a loan borrowing more than the lender has offered.

[samuraii77]

I said in the pool which means Aave pool (as available liquidity), not Size.

[gesha17]

Ah, I see what you meant. Got confused by the example.

[hansfriese]

It is not a duplicate of #152, but I believe it is not a valid Medium risk.

• QA is appropriate since an attacker would lose a non-negligible amount of funds.
• It seems illogical that the pool would not utilize the increased funds (though I did not check the logic of AAVE v3 in detail).

Now, the function passes but the actual aToken balance of the pool is still 90 as those 10 were directly sent

I will invalidate this issue as the warden hasn't submitted a QA report.

[c4-judge]

hansfriese marked the issue as not a duplicate

[c4-judge]

hansfriese changed the severity to QA (Quality Assurance)

[c4-judge]

hansfriese marked the issue as grade-c