sherlock-audit / 2024-04-teller-finance-judging

10 stars 9 forks source link

bughuntoor - In case the user's interest is more than their principal, they can wait to `liquidateDefaultedLoanWithIncentive` for profit #30

Closed sherlock-admin3 closed 5 months ago

sherlock-admin3 commented 6 months ago

bughuntoor

medium

In case the user's interest is more than their principal, they can wait to liquidateDefaultedLoanWithIncentive for profit

Summary

If the user has taken a loan from LenderCommitmentGroup_Smart.sol and the interest is more than their principal, they can wait to liquidateDefaultedLoanWithIncentive for profit

Vulnerability Detail

When calling liquidateDefaultedLoanWithIncentive within LenderCommitmentGroup_Smart, the accrued fees are ignored and user has to only pay based on their principal amount.

    function liquidateDefaultedLoanWithIncentive(
        uint256 _bidId,
        int256 _tokenAmountDifference
    ) public bidIsActiveForGroup(_bidId) {
        uint256 amountDue = getAmountOwedForBid(_bidId, false);

        uint256 loanDefaultedTimeStamp = ITellerV2(TELLER_V2)
            .getLoanDefaultTimestamp(_bidId);

        int256 minAmountDifference = getMinimumAmountDifferenceToCloseDefaultedLoan(
                amountDue,
                loanDefaultedTimeStamp
            );

        require(
            _tokenAmountDifference >= minAmountDifference,
            "Insufficient tokenAmountDifference"
        );

The problem occurs in the case where the user's interest is more than their principal, meaning that as soon as liquidateDefaultedLoanWithIncentive becomes callable, they can call it and pay 2x their principal (which would still be less than what they would pay) and claim back their collateral.

Impact

Users can liquidate themselves for profit.

Code Snippet

https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L422C1-L439C11

Tool used

Manual Review

Recommendation

add proper incentivization for users to repay their loans instead of liquidating themselves.

nevillehuang commented 5 months ago

request poc

This issue lacks details to prove how the state can be reached without risking prior liquidation?

sherlock-admin4 commented 5 months ago

PoC requested from @spacegliderrrr

Requests remaining: 10

ethereumdegen commented 5 months ago

This doesnt make sense . While your statement is true, game theory suggests that someone else would have liquidated the loan before them if it was profitable to do so (assuming perfect knowledge and access)

Furthermore, as soon as liquidateDefaultedLoanWithIncentive becomes callable, there is a 800% penalty on it which decreases slowly overtime

spacegliderrrr commented 5 months ago

Escalate

Should be dup of #121

sherlock-admin3 commented 5 months ago

Escalate

Should be dup of #121

You've created a valid escalation!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

nevillehuang commented 5 months ago

This issue is arguable. The majority of the issue's description (Summary, Impact and Recommendation) is incorrect except the following sentence:

When calling liquidateDefaultedLoanWithIncentive within LenderCommitmentGroup_Smart, the accrued fees are ignored and user has to only pay based on their principal amount.

One can argue the impact and rootcause is wrongly described, given the watson highlighted the problem as the following:

The problem occurs in the case where the user's interest is more than their principal, meaning that as soon as liquidateDefaultedLoanWithIncentive becomes callable

I will leave it to @cvetanovv to decide, but I personally believe this issue should remain invalid based on the following

In addition to this, there is a submission D which identifies the core issue but does not clearly describe the impact or an attack path. Then D is considered low.

cvetanovv commented 5 months ago

I agree with @nevillehuang comment.

The root of this issue is that the user can liquidate himself for profit, which is fundamentally different from the root cause of #121. What they have in common is that liquidation does not account for the interest. Everything else is different, and as the Lead Judge has pointed out, under Sherlock rules, this issue is invalid.

Planning to reject the escalation and leave the issue as is.

Evert0x commented 5 months ago

Result: Invalid Unique

sherlock-admin4 commented 5 months ago

Escalations have been resolved successfully!

Escalation status: