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

Builder can halve the interest paid to a community owner due to arithmetic rounding #180

Open
code423n4 opened this issue Aug 6, 2022 · 0 comments
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") valid

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L685-L686

Vulnerability details

Impact

Due to arithmetic rounding in returnToLender(), a builder can halve the APR paid to a community owner by paying every 1.9999 days. This allows a builder to drastically decrease the amount of interest paid to a community owner, which in turn allows them to advertise very high APR rates to secure funding, most of which they will not pay.

This issue occurs in the calculation of noOfDays in returnToLender() which calculates the number of days since interest has last been calculated. If a builder repays a very small amount of tokens every 1.9999 days, then the noOfDays will be rounded down to 1 days however lastTimestamp is updated to the current timestamp anyway, so the builder essentially accumulates only 1 day of interest after 2 days.

I believe this is high severity because a community owner can have a drastic decrease in interest gained from a loan which counts as lost rewards. Additionally, this problem does not require a malicious builder because if a builder pays at a wrong time, the loaner receives less interest anyway.

Proof of Concept

  1. A community owner provides a loan of 500_000 tokens to a builder with an APR of 10% (ignoring treasury fees)
  2. Therefore, the community owner will expect an interest of 136.9 tokens per day (273.9 per 2 days)
  3. A builder repays 0.000001 tokens at lastTimestamp + 2*86400 - 1
  4. noOfDays rounds down to 1 thereby accumulating 500_000 * 100 * 1 / 365000 = 136 tokens for 2 days
  5. Therefore, the community owner only receives 5% APR with negligible expenses for the builder

Tools Used

VS Code

Recommended Mitigation Steps

There are two possible mitigations:

  1. Add a scalar to noOfDays so that any rounding which occurs is negligible
    i.e.
        uint256 _noOfDays = (block.timestamp -
            _communityProject.lastTimestamp) * SCALAR / 86400; // 24*60*60


        /// Interest formula = (principal * APR * days) / (365 * 1000)
        // prettier-ignore
        uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                _noOfDays /
                365000 /
                SCALAR;
  1. Remove the noOfDays calculation and calculate interest in one equation which reduces arithmetic rounding
uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                (block.timestamp -
            _communityProject.lastTimestamp) /
                365000 /
                86400;
@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Aug 6, 2022
code423n4 added a commit that referenced this issue Aug 6, 2022
@zgorizzo69 zgorizzo69 added sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") and removed sponsor disputed Sponsor cannot duplicate the issue, or otherwise disagrees this is an issue labels Aug 6, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working sponsor confirmed Sponsor agrees this is a problem and intends to fix it (OK to use w/ "disagree with severity") valid
Projects
None yet
Development

No branches or pull requests

3 participants