Skip to content
This repository has been archived by the owner on Jan 7, 2024. It is now read-only.

n33k - Vault: _update_debt does not accrue interest #167

Open
sherlock-admin opened this issue Jul 5, 2023 · 9 comments
Open

n33k - Vault: _update_debt does not accrue interest #167

sherlock-admin opened this issue Jul 5, 2023 · 9 comments
Labels
Escalation Resolved This issue's escalations have been approved/rejected Fix Submitted Fix to the issue has been submitted Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed

Comments

@sherlock-admin
Copy link
Contributor

n33k

medium

Vault: _update_debt does not accrue interest

Summary

_update_debt call _debt_interest_since_last_update to accrue interest but _debt_interest_since_last_update always return 0 in _update_debt.

Vulnerability Detail

_update_debt sets self.last_debt_update[_debt_token] to block.timestamp and then calls _debt_interest_since_last_update.

def _update_debt(_debt_token: address):
    ....
    self.last_debt_update[_debt_token] = block.timestamp
    ....
    self.total_debt_amount[_debt_token] += self._debt_interest_since_last_update(
        _debt_token
    )

_debt_interest_since_last_update always returns 0. because block.timestamp - self.last_debt_update[_debt_token] is always 0.

def _debt_interest_since_last_update(_debt_token: address) -> uint256:
    return (
        (block.timestamp - self.last_debt_update[_debt_token])
        * self._current_interest_per_second(_debt_token)
        * self.total_debt_amount[_debt_token]
        / PERCENTAGE_BASE
        / PRECISION
    )

Impact

Debt fee is not accrued.

Code Snippet

https://github.com/sherlock-audit/2023-06-unstoppable/blob/main/unstoppable-dex-audit/contracts/margin-dex/Vault.vy#L1050-L1076

Tool used

Manual Review

Recommendation

Call _debt_interest_since_last_update then update last_debt_update.

@Unstoppable-DeFi
Copy link

@sherlock-admin2 sherlock-admin2 added the Reward A payout will be made for this issue label Jul 19, 2023
@0x00ffDa
Copy link

Escalate for 10 USDC.

I believe this should be high severity.
This flaw results in definite loss of funds for the protocol and LP providers: zero revenue from all debts, no special circumstances needed.

@sherlock-admin2
Copy link

Escalate for 10 USDC.

I believe this should be high severity.
This flaw results in definite loss of funds for the protocol and LP providers: zero revenue from all debts, no special circumstances needed.

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.

@sherlock-admin sherlock-admin added the Escalated This issue contains a pending escalation label Jul 20, 2023
@141345
Copy link
Collaborator

141345 commented Jul 23, 2023

Recommendation:
keep the original judging.

Escalate for 10 USDC.

I believe this should be high severity. This flaw results in definite loss of funds for the protocol and LP providers: zero revenue from all debts, no special circumstances needed.

The loss is on interest, not big loss in principal, the amount might not be considered "material loss of funds".

I think medium is appropriate.

@Nabeel-javaid
Copy link

IMO, high is appropriate is it is bricking the core functionality of interest. There is no point of lending when there is no interest on it.

@Unstoppable-DeFi Unstoppable-DeFi added the Fix Submitted Fix to the issue has been submitted label Jul 27, 2023
@0xpinky
Copy link

0xpinky commented Aug 4, 2023

interest is core concept in lend and borrow, flawed interest updates would brick one of the core functions. when we look at this over a period of time, the loss would have incurred is huge..
High is appropriate.

@hrishibhat hrishibhat added High A valid High severity issue and removed Medium A valid Medium severity issue labels Aug 4, 2023
@hrishibhat
Copy link
Contributor

Result:
High
Has duplicates
Considering this issue a valid high for two reasons:

  • It breaks the core functionality of accruing interest on debt.
  • Leading loss of revenue for protocol and LP.

@sherlock-admin2 sherlock-admin2 removed the Escalated This issue contains a pending escalation label Aug 4, 2023
@sherlock-admin sherlock-admin added the Escalation Resolved This issue's escalations have been approved/rejected label Aug 4, 2023
@sherlock-admin2
Copy link

Escalations have been resolved successfully!

Escalation status:

@maarcweiss
Copy link
Member

Fixed by calling last_debt_update after _debt_interest_since_last_update

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Escalation Resolved This issue's escalations have been approved/rejected Fix Submitted Fix to the issue has been submitted Has Duplicates A valid issue with 1+ other issues describing the same vulnerability High A valid High severity issue Reward A payout will be made for this issue Sponsor Confirmed The sponsor acknowledged this issue is valid Will Fix The sponsor confirmed this issue will be fixed
Projects
None yet
Development

No branches or pull requests

9 participants