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

Community can lose interest because interest was calculated by days instead of seconds #221

Closed
code423n4 opened this issue Aug 6, 2022 · 1 comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working duplicate This issue or pull request already exists valid

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-08-rigor/blob/main/contracts/Community.sol#L684-L694

Vulnerability details

Impact

Community's owner lose amount of interest from project (up to half of total interest)

Proof of concept

When builder repay any loan amount by function repayLender() (or community call function lendToProject()), function claimInterest() in contract Community will be called and then calculate the interest in function returnToLender()
It will calculate number of days difference current and last timestamp

        uint256 _noOfDays = (block.timestamp -
            _communityProject.lastTimestamp) / 86400; // 24*60*60

Here, the calculation skipped (block.timestamp - _communityProject.lastTimestamp) % 86400 seconds. The more number of calls repayLender() and lendToProject(), the more time was skipped.
After that:

uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                _noOfDays /
                365000;

When _unclaimedInterest > 0 (means _noOfDays > 0), variable lastTimestamp will be updated in function claimInterst(). Then amount of redundant seconds will be accrued (can up to a half of total time).

Example scenerio:

Builder will call repayLender() (any amount > 0, such as 1 wei) continuously with number of seconds difference is smaller and close to 86400 * 2. After 1 year (365 days), the total interest was calculated by about 183 days, then community's owner has lost almost half of total interest which should be claimed from this project.

Tools Used

Manual review

Recommended Mitigation Steps

Should calculate the interest based unit seconds instead of days.

uint256 _differentTime = block.timestamp - _communityProject.lastTimestamp;

uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                differentTime /
                (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
@horsefacts
Copy link

Duplicate of #180

@itsmetechjay itsmetechjay added the duplicate This issue or pull request already exists label Aug 18, 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 duplicate This issue or pull request already exists valid
Projects
None yet
Development

No branches or pull requests

4 participants