The check for available liquidity is incorrect if the token was ever lent

[hats-bug-reporter]

Github username: @GalloDaSballo
Submission hash (on-chain): 0x39e6bdd72c6d292861d39347759d05434e7dfa37f4bbf1c8f6089f093fe6de87
Severity: low severity

Description:
Description\

VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/packages/contracts/contracts/protocol/lendingpool/LendingPoolCollateralManager.sol

Lines 184 to 187 in f14637d

	if (!receiveAToken) {
	uint256 currentAvailableCollateral = IERC20(vars.collateralAsset)
	.balanceOf(vars.collateralAToken);
	currentAvailableCollateral = currentAvailableCollateral.add(IAToken(vars.collateralAToken).getStakedAmount());

        if (!receiveAToken) {
            uint256 currentAvailableCollateral = IERC20(vars.collateralAsset)
                .balanceOf(vars.collateralAToken);
            currentAvailableCollateral = currentAvailableCollateral.add(IAToken(vars.collateralAToken).getStakedAmount()); // Looks off

Looks wrong cause totalSupply is cognizant of the index
vs staked doesn't grow over time

Can be verified by checking the totalSupply on any live contract such as the CRV Market on Mainnet
https://etherscan.io/address/0x8dAE6Cb04688C62d939ed9B68d32Bc62e49970b1

Which over a few seconds grows, e.g.

298530189024582799736699096 uint256
298530189772748324764861196 uint256

This will make it so that the contract will return

Attack Scenario\

This creates a scenario where, if enough tokens are borrowed and the totalSupply has grown sufficiently (a bunch of interest yet to be paid), the check will pass, but liquidations will not work as intended as the AToken will not have sufficient tokens to perform the liquidation

The TX should revert at Atoken.burn due to lack of liquidity

VMEX-0x050183b53cf62bcd6c2a932632f8156953fd146f/packages/contracts/contracts/protocol/tokenization/AToken.sol

Lines 165 to 166 in f14637d

	IERC20(_underlyingAsset).safeTransfer(receiverOfUnderlying, amount);

        IERC20(_underlyingAsset).safeTransfer(receiverOfUnderlying, amount);

[ksyao2002]

Again, tokens with staking rewards will not be lent out #33 #32. With the two cases

1. Liquidated token is also borrowable (this means that totalStakedAmount is zero so it doesn't affect the calculations)
2. Liquidated token is not borrowable (this means that index never grows so the attack scenario doesn't apply)

Please let me know if there is anything I am missing.

[GalloDaSballo]

What I would add is that for reasons similar to #57 if a token started as borrowable, and then a staking contract is added, a migration plan of some sort will need to be enacted, as otherwise the older tranches will run into this bug + the one from #57

While newer pools will have the aToken index to 1-1 and for that reason all checks will pass

[ksyao2002]

That's a good consideration. I don't think we would have any tokens that we allow for borrowing, and then enable staking rewards for, since borrowing is only for the base tokens (like USDC) that don't have staking systems, and staking systems are generally for lp tokens that we won't ever consider for borrowing.

[0xcuriousapple]

hmm, there are additional issues with allowing lp tokens as borrowable.
good to know that you are not looking to make them borrowable.
the whole accounting for current rewards distribution doesn't work once it's borrowable, one could easily game it by doing a borrow and deposit of the lp tokens in loop

[ksyao2002]

Yeah, we coded it with the intent that they are not borrowable. It drastically changes accounting logic if they are borrowable.