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

CapsLibrary.validateVariablePoolHasEnoughLiquidity() always revert #35

Closed
c4-bot-2 opened this issue Jun 30, 2024 · 4 comments
Closed
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 🤖_primary AI based primary recommendation 🤖_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

@c4-bot-2
Copy link
Contributor

Lines of code

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

Vulnerability details

After a user attempts lending or borrowing, the function CapsLibrary.validateVariablePoolHasEnoughLiquidity() is called to ensure the Aave pool has enough liquidity for the current borrower to withdraw USDC. However, it checks the wrong address and reverts.

Impact

No user can utilize the lending or borrowing functions until the contracts are upgraded to fix the issue.
No financial loss incurred to user.

Proof of Concept

In SizeStorage, the configuration variablePool points to the AAVE V3 main pool (0x87870Bca3F3fD6335C3F4ce8392D69350B4fA4E2), which hold tokens.

When CapsLibrary tries to get the underlyingBorrowToken (USDC) balance of variablePool, it always returns 0, as shown below:

    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {/
        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));//@get USDC balance of aave pool
        if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }//@audit H did query state query wrong pool here? state.data.variablePool is main aave Pool. it does not hold any token.
    }

This function is supposed to point to the Aave pool pair ETH/USDC, as seen in DepositTokenLibrary.sol:
https://github.com/code-423n4/2024-06-size/blob/8850e25fb088898e9cf86f9be1c401ad155bea86/src/libraries/DepositTokenLibrary.sol#L55-L56

Because aEthUSDC contract holds the actual USDC for user withdrawals, not the main pool.

Tools Used

Recommended Mitigation Steps

Point to correct address

    function validateVariablePoolHasEnoughLiquidity(State storage state, uint256 amount) public view {
        // uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(address(state.data.variablePool));//@USDC balance
        //fixed to working with fork test
        address aToken = state.data.variablePool.getReserveData(address(state.data.underlyingBorrowToken)).aTokenAddress;
        uint256 liquidity = state.data.underlyingBorrowToken.balanceOf(aToken);//@USDC balance
        if (liquidity < amount) {
            revert Errors.NOT_ENOUGH_BORROW_ATOKEN_LIQUIDITY(liquidity, amount);
        }
    }

Assessed type

Error

@c4-bot-2 c4-bot-2 added 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 labels Jun 30, 2024
c4-bot-4 added a commit that referenced this issue Jun 30, 2024
@c4-bot-12 c4-bot-12 added the 🤖_10_group AI based duplicate group recommendation label Jul 2, 2024
@c4-bot-13 c4-bot-13 added the 🤖_primary AI based primary recommendation label Jul 2, 2024
@howlbot-integration howlbot-integration bot added sufficient quality report This report is of sufficient quality duplicate-114 labels Jul 4, 2024
@C4-Staff
Copy link

C4-Staff commented Jul 8, 2024

CloudEllie marked the issue as duplicate of #218

@c4-judge
Copy link
Contributor

hansfriese changed the severity to 3 (High Risk)

@c4-judge c4-judge added 3 (High Risk) Assets can be stolen/lost/compromised directly upgraded by judge Original issue severity upgraded from QA/Gas by judge satisfactory satisfies C4 submission criteria; eligible for awards and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Jul 11, 2024
@c4-judge
Copy link
Contributor

hansfriese marked the issue as satisfactory

@c4-judge
Copy link
Contributor

hansfriese changed the severity to 2 (Med Risk)

@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 upgraded by judge Original issue severity upgraded from QA/Gas by judge labels Jul 21, 2024
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 🤖_primary AI based primary recommendation 🤖_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

5 participants