Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Interest accrued could be zero for small decimal tokens #10

Open
code423n4 opened this issue Mar 30, 2022 · 1 comment
Open

Interest accrued could be zero for small decimal tokens #10

code423n4 opened this issue Mar 30, 2022 · 1 comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/sublime-finance/sublime-v1/blob/46536a6d25df4264c1b217bd3232af30355dcb95/contracts/PooledCreditLine/PooledCreditLine.sol#L1215-L1221

Vulnerability details

Details & Impact

Interest is calculated as

(_principal.mul(_borrowRate).mul(_timeElapsed).div(YEAR_IN_SECONDS).div(SCALING_FACTOR));

It is possible for the calculated interest to be zero for principal tokens with small decimals, such as EURS (2 decimals). Accumulated interest can therefore be zero by borrowing / repaying tiny amounts frequently.

Proof of Concept

Assuming a borrow interest rate of 5% (5e17) and principal borrow amount of 10_000 EURS (10_000 * 1e2 = 1_000_000), the interest rate calculated would be 0 if principal updates are made every minute (around 63s).

// in this example, maximum duration for interest to be 0 is 63s
1_000_000 * 5e17 * 63 / (86400 * 365) / 1e18 = 0.99885 // = 0

While plausible, this method of interest evasion isn’t as economical for tokens of larger decimals like USDC and USDT (6 decimals).

Recommended Mitigation Steps

Take caution when allowing an asset to be borrowed. Alternatively, scale the principal amount to precision (1e18) amounts.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Mar 30, 2022
code423n4 added a commit that referenced this issue Mar 30, 2022
@ritik99 ritik99 added the disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments) label Apr 6, 2022
@ritik99
Copy link
Collaborator

ritik99 commented Apr 6, 2022

Tokens are whitelisted to ensure precision issues would not occur. Hence the issue is improbable and doesn't occur for widely used tokens as the decimals are generally higher.

Since there is no direct attack path (the steps required for this to occur would be: the token would first have to be whitelisted -> a loan request created using it -> lenders supply sufficient liquidity for this request to go active) and is, in essence, a value leak, we would suggest reducing the severity of the issue to (1) Low / (2) Medium

@HardlyDifficult HardlyDifficult added 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value and removed 3 (High Risk) Assets can be stolen/lost/compromised directly labels Apr 25, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value bug Something isn't working disagree with severity Sponsor confirms validity, but disagrees with warden’s risk assessment (sponsor explain in comments)
Projects
None yet
Development

No branches or pull requests

4 participants