Boost _ Folks Finance 33970 - [Smart Contract - Medium] User deposits can be blocked
Submitted on Sat Aug 03 2024 01:56:26 GMT-0400 (Atlantic Standard Time) by @JCN2023 for Boost | Folks Finance
Report ID: #33970
Report type: Smart Contract
Report severity: Medium
Target: https://testnet.snowtrace.io/address/0x2cAa1315bd676FbecABFC3195000c642f503f1C9
Impacts:
Griefing (e.g. no profit motive for an attacker, but damage to the users or the protocol)
Description
Brief/Intro
Currently there are two loanTypeIds
(1, 2) for each poolId
. Therefore, there are two loanPools
available for each pool, since a loanPool
is defined as loanPool = (loanTypeId, poolId)
. Activity will vary across the loanPools
for a specific poolId
and thus the collateralRewardIndex
for these loanPools
will vary (one will be greater than the other). At the time of writing this report, loanTypeId 1
for poolIds
128 & 129 (USDC & AVAX) point to a loanPool = (1, 128/129)
that has a smaller collateralRewardIndex
compared to loanPool = (2, 128/129)
. This, coupled with the fact that loanIds
are reusable (can be created and deleted) and can hold values from previous usage, allows a bad actor to block users from depositing into certain pools via the following actions:
Create loan using user's specified
loanId
(seen in mempool), but specifying aloanTypeId
that points to aloanPool
that has a largercollateralRewardIndex
Deposit into
loanPool
so that the local_userLoans[loanId].collaterals[poolId].rewardIndex
gets set to the globalloanPool.reward.collateralRewardIndex
valueWithdraw all funds from pool
Delete loan to free up
loanId
, but local index forloanId
andpoolId
persistsUser creates loan with
loanId
forloanTypeId
corresponding to smaller global index for differentloanPool
User attempts to deposit into
loanPool
, but tx reverts due to underflow when updating user's collateral rewards, sincelocal_index > global_index
Conditions: The user's createLoan
transaction has to specify a loanTypeId
that results in a loanPool
(when taking into account poolIds
) which has a smaller collateralRewardIndex
compared to other loanPools
for other loanTypeIds
(used by bad actor).
Bug Description
A loan is identified by a loanId
and holds the following data:
When a loan is deleted, it is first verified that the loan is empty but only the collateral and borrow positions arrays are validated to be empty (see line 66 below). Additionally, the delete
keyword is used to reset all of the values in the UserLoan
struct for this loanId
(see line 69 below). However, the delete
keyword does not have an effect on mappings and therefore its possible that the collaterals
mapping for a loan will still hold UserLoanCollateral
values that were previously stored for a specified poolId
key, even after the loan is deleted.
LoanManagerState::_isUserLoanEmpty
To understand why this can be an issue, we will observe the flow for deposit
transaction:
LoanManagerLogic::executeDeposit
RewardLogic::updateUserCollateralReward
As we can see above, when updating the user's collateral rewards during deposits
, the global index for a loanPool
is defined by a loanTypeId
and poolId
pair. I.e. loanPool = (1, 128)
will have a different global index compared to loanPool = (2, 128)
. However, the local index for the user is defined only by the poolId
. Therefore, this local index can be implicitly pointing to a loanPool
with loanTypeId
of 1 or 2. It is assumed that the local index will be consistent with the UserLoan.loanTypeId
value that is specified for the created loan (and therefore consistent with the global index for a loanTypeId
, poolId
pair). However, we have already seen that the local index for a loan can persist after deletion. Therefore, the following situation can arise:
global index for
loanPool = (1, 128)
isx
global index for
loanPool = (2, 128)
isx + 1
Loan is created with
loanId
andloanTypeId = 2
.poolId = 128
is deposited into, setting the loan's local index forpoolId = 128
tox + 1
loan is emptied (all collateral withdrawn) and deleted
New loan is created with
loanId
andloanTypeId = 1
Current local index for loan and
poolId = 128
isx + 1
, but global index for(loanTypeId, poolId)
isx
After the above situation occurs, when the user attempts to deposit into poolId = 128
with the new loan, the transaction will revert on line 561 in MathUtils.sol
. This is due to the fact that the global index for the loanPool = (1, 128)
is x
, which is less than the local index for the new loan and poolId (x + 1
). Thus, the rewardIndexAtT - rewardIndexAtT_1
operation will result in an underflow.
Impact
A bad actor is able to block users from depositing into certain pools. This exploit can only occur against users who are creating loans with loanTypeIds
that correspon