The repayment is made after the market state is update in the `WildcatMarket.repayOutstandingDebt` and `WildcatMarket.repayDelinquentDebt` functions thus putting the borrower at a disadvantage #72
In the WildcatMarket.repay function and the WildcatMarketWithdrawals.repayAndProcessUnpaidWithdrawalBatches the repyament of the borrower is made before the market state is updated. The natspec comments given for the repayAndProcessUnpaidWithdrawalBatches function is as follows:
// Repay before updating state to ensure the paid amount is counted towards
// any pending or unpaid withdrawals.
Hence according to the above natspec comments it is clear that repayment needs to be done prior to market updates since the repaid amount can be used to cover for pending or unpaid withdrawals. This is how the protocol is designed to operate.
But the issue is in the WildcatMarket.repayOutstandingDebt and WildcatMarket.repayDelinquentDebt functions the market state update happens before the funds are repaid to the market contract. Hence these repaid amounts are not used to cover for the pending or unpaid withdrawals. Hence there is a discrepancy in the execution flow of the above different repay mechanisms.
For certain repayments the transferred amount is not accounted for the immediate repayment towards the pending or unpaid withdrawals. Where as for the other repayments the transferred amount is accounted for the immediate repayment towards the pending or unpaid withdrawals.
Impact
Let's consider the borrower is in the deliquent state for his market. He decides to repay the deliquent debt by calling the WildcatMarket.repayDelinquentDebt function. Though he repays the deliquent debt amount it is not immediately accounted in the contract state since the market state update happens prior to the repayment. His repayment will only take effect when the market state is updated for a later transaction. During this period he could exceed the deliquent grace period or could add additional time on this deliquent grace duration which is either way disadvantageous to the borrower.
As a result the borrower will have to incur deliquent interest fee which he shouldn't have paid in the first place if the repayment was done prior to the market state update in the WildcatMarket.repayDelinquentDebt function.
Proof of Concept
asset.safeTransferFrom(msg.sender, address(this), amount);
emit_DebtRepaid(msg.sender, amount);
MarketState memory state = _getUpdatedState();
Hence it is recommended to ensure that the repayment is done prior to the market state update in the WildcatMarket.repayDelinquentDebt and WildcatMarket.repayOutstandingDebt functions. This will ensure that repayment done by the borrower is immediately accounted for the repayment towards the pending or unpaid withdrawals. Thus ensuring the states of the borrower are not adversely affected.
Lines of code
https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L205-L208 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L186-L187 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L179-L180 https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L172
Vulnerability details
Vulnerability Description:
In the
WildcatMarket.repay
function and theWildcatMarketWithdrawals.repayAndProcessUnpaidWithdrawalBatches
therepyament of the borrower
is made before the market state is updated. The natspec comments given for therepayAndProcessUnpaidWithdrawalBatches
function is as follows:Hence according to the above natspec comments it is clear that repayment needs to be done prior to market updates since the
repaid amount
can be used to cover for pending or unpaid withdrawals. This is how the protocol is designed to operate.But the issue is in the
WildcatMarket.repayOutstandingDebt
andWildcatMarket.repayDelinquentDebt
functions themarket state update
happensbefore the funds are repaid
to the market contract. Hence these repaid amounts are not used to cover for thepending or unpaid withdrawals
. Hence there is a discrepancy in the execution flow of the above different repay mechanisms.For certain repayments the transferred amount is not accounted for the immediate repayment towards the pending or unpaid withdrawals. Where as for the other repayments the transferred amount is accounted for the immediate repayment towards the pending or unpaid withdrawals.
Impact
Let's consider the borrower is in the
deliquent state
for his market. He decides to repay thedeliquent debt
by calling theWildcatMarket.repayDelinquentDebt
function. Though he repays thedeliquent debt
amount it is not immediately accounted in the contract state since themarket state
update happens prior to therepayment
. His repayment will only take effect when themarket state
is updated for a later transaction. During this period he could exceed thedeliquent grace period
orcould add additional time on this deliquent grace duration
which is either way disadvantageous to the borrower.As a result the borrower will have to incur
deliquent interest fee
which he shouldn't have paid in the first place if therepayment
was done prior to themarket state
update in theWildcatMarket.repayDelinquentDebt
function.Proof of Concept
https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L205-L208
https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L186-L187
https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L179-L180
https://github.com/code-423n4/2024-08-wildcat/blob/main/src/market/WildcatMarket.sol#L172
Recommended Mitigation Steps
Hence it is recommended to ensure that the
repayment
is done prior to themarket state
update in theWildcatMarket.repayDelinquentDebt
andWildcatMarket.repayOutstandingDebt
functions. This will ensure that repayment done by the borrower is immediately accounted for the repayment towards the pending or unpaid withdrawals. Thus ensuring the states of the borrower are not adversely affected.Assessed type
Other