Submitted by WoolCentaur, also found by SBSecurity and AM
When liquidating a user, _withdrawOrAllocateSharesLiquidation() can be called which checks if a pool is large enough to pay out the liquidator, and if not, then the liquidator is allocated shares that can be used to withdraw at a later time when the pool is large enough. There are two scenarios in which the pool will not be large enough to pay out the liquidator:
Note: If there are some tokens available but not enough to cover the entire withdraw amount, then those tokens are transferred, dropping the total pool to 0, and the rest are allocated as shares.
However, if these scenarios were ever to arise, the _withdrawOrAllocateSharesLiquidation() function would not work as expected. This is due to how it calculates the lending shares (totalPoolInShares) of the total pool:
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L493
When liquidating, it will calculate the totalPoolInShares like this (with amount being the total pool):
However, calculating the lending shares this way will cause the _compareSharePrices() to revert on the syncPool modifier. Specifically the check on
This is because _maxSharePrice is passed in as false to calculateLendingShares(), which subtracts one.
https://github.com/code-423n4/2024-02-wise-lending/blob/79186b243d8553e66358c05497e5ccfd9488b5e2/contracts/WiseCore.sol#L497
However, because we are withdrawing, it should be set to true so that it adds one. In all other areas where calculateLendingShares() is being used, the functions that focus on withdrawing have _maxSharePrice: true and the depositing functions have _maxSharePrice: false:
withdrawOnBehalfExactAmount on WiseLending.sol.
_preparationsWithdraw on MainHelper.sol.
_handleDeposit on WiseCore.sol.
When _maxSharePrice is set to true, the _validateParameter() check will pass.
Even if the _validateParameter() check did not exist, if _maxSharePrice is left as-is (set to false), any subsequent liquidations through _withdrawOrAllocateSharesLiquidation() would revert with panic due to an underflow. This is because (as stated above), liquidating when the pool is not large enough to pay out the liquidator will drop the total pool to 0 and issue the remainder as shares. Therefore, any subsequent calls to liquidate when the total pool is 0 will underflow:
When a particular receiving token is desired and _withdrawOrAllocateSharesLiquidation() is called, the liquidation will always revert if the total pool is not large enough to cover the withdraw amount. This defeats the purpose of _withdrawOrAllocateSharesLiquidation(), as stated in the NatSpec:
This will lead to frustrated users who desire a particular receiving token. The level of frustration will be even higher if the reason this function reverts is because the total pool is borrowed out. This would lead to a very high APY, thus a much higher desire to receive the particular token. Additionally, the borrowers in these particular situations will not be liquidated even though they should be. Ultimately, this will lead to loss of faith in the protocol.
However, this can be easily avoided by just passing in a different receiving token that is large enough to payout the withdraw amount, which is why this is a medium level issue.
Copy and paste this foundry test into the /contracts folder and run forge test --fork-url mainnet --match-path ./contracts/WoolCentaurLiquidationTest.t.sol --match-test testLiquidateMockPool -vvvv.  As the code is, the test will fail.  To have it pass, change the bool here
to true.
Foundry
Change the _maxSharePrice under calculateLendingShares on _withdrawOrAllocateSharesLiquidation() to true.
Error
vm06007 (Wise Lending) commented via duplicate issue #238:
vonMangoldt (Wise Lending) commented via duplicate issue #238:
Trust (judge) commented:
Wise Lending commented:
