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

If builder is replaying the lender not so long after getting funded the builder can get out of paying intrest on borrowed funds #136

Closed
code423n4 opened this issue Aug 5, 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/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L455
https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L824
https://github.com/code-423n4/2022-08-rigor/blob/5ab7ea84a1516cb726421ef690af5bc41029f88f/contracts/Community.sol#L668

Vulnerability details

Impact

If builder makes a project,then a day later the builder gets everything done and wants to repay the lender.RepaysLender in that function it calls _reduceDebt.claimInterest.returnToLender() and if noOfDays = 0 your unclaimedInterset =0 it lowers the builders Interest to 0 and you still have _lentAmount left over causing tokens that the builder borrowed arent getting intrest on it.

Proof of Concept

1.attacker calls repayLender which calls _reduceDebt then calls claimInterest then returnToLender
note: the diffrent in the timestamp is one day august 6 to the 5 at the same time of the day.
block.timestmap= (1659817999- 1659732167) / 86400 = 0 because of percsion error
unclaimedInterest = 0 and totalInsterest =5 (lets say 5 )


returnToLender function


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

        /// Interest formula = (principal * APR * days) / (365 * 1000)
        // prettier-ignore
        uint256 _unclaimedInterest = 
                _lentAmount *
                _communities[_communityID].projectDetails[_project].apr *
                _noOfDays /
                365000;

        // Old (already rTokens claimed) + new interest
        uint256 _totalInterest = _unclaimedInterest +
            _communityProject.interest;

claimInterest function

  • the if statement never gets called because unclaimedInterest = 0 = _interestEarned
      if (_interestEarned > 0) {
            // If any new interest is to be claimed.

            // Increase total interest with new interest to be claimed.
            _communityProject.interest = _interest;


            // Update lastTimestamp to current time.
            _communityProject.lastTimestamp = block.timestamp;

            // Burn _interestEarned amount wrapped token to lender
            _wrappedToken.mint(_lender, _interestEarned);

repayLender function

  • attacker has alot more repayAmount (100) then _interest(5)`
  • _lentAndInterest = 100 + 5= 115 which is greater then 100
  • and then _interest =0 and _lentAmount=15
        uint256 _lentAmount = _communityProject.lentAmount;
        uint256 _interest = _communityProject.interest;

        if (_repayAmount > _interest) {
            // If repayment amount is greater than interest then
            // set lent = lent + interest - repayment.
            // And set interest = 0.
            uint256 _lentAndInterest = _lentAmount + _interest;

            // Revert if repayment amount is greater than sum of lent and interest.
            require(_lentAndInterest >= _repayAmount, "Community::!Liquid");
            _interest = 0;
            _lentAmount = _lentAndInterest - _repayAmount;
        } else {
            // If repayment amount is lesser than interest, then set
            // interest = interest - repayment
            _interest -= _repayAmount;
        }

        // Update community  project details
        _communityProject.lentAmount = _lentAmount;
        _communityProject.interest = _interest;
  • attacker/builder at the end gives back the debt tokens but hes not done borrowing funds. This can make the issue worse because there is no pressure or insurance that the community is getting there funds back.

At the end of this brief window of time an builder of the project can still have borrowable assets that arent geting intrested on which can be considerd stealing funds.
The reason am rating this as medium is because the attack window is only a day or so and to only have the borrow asset ready to be gived back in a day is not easy.

Tools Used

vim

Recommended Mitigation Steps

  • in the returnToLender function add a check to make sure that _noOfDays is > 0;
require(_noOfDays > 0);
@code423n4 code423n4 added 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 labels Aug 5, 2022
code423n4 added a commit that referenced this issue Aug 5, 2022
@parv3213
Copy link
Collaborator

Duplicate of #180

@parv3213 parv3213 marked this as a duplicate of #180 Aug 17, 2022
@itsmetechjay itsmetechjay added the duplicate This issue or pull request already exists label Aug 18, 2022
@jack-the-pug jack-the-pug added 3 (High Risk) Assets can be stolen/lost/compromised directly valid and removed 2 (Med Risk) Assets not at direct risk, but function/availability of the protocol could be impacted or leak value labels Aug 27, 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