code-423n4 / 2024-03-ondo-finance-findings

5 stars 6 forks source link

Users can increase their balances by transfering shares #318

Closed c4-bot-7 closed 6 months ago

c4-bot-7 commented 6 months ago

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L501-L519

Vulnerability details

Impact

Users will increase their balances just by transferring funds a malicious user can use this to have more balance with multiple EOA that transfer rOUSG and shares in order to steal funds.

Proof of Concept

Alice mint some rOusg transfer an amount to bob and then transfer 1 share it will increase the sum of both balances. Here is a test that proves that :

function test_user_can_increaseTheirShares() public {
        //We set The price at 112 $
     oracleCheckHarnessOUSG.setPrice(112000000000000000008);
 vm.prank(OUSG_GUARDIAN);
 //set the mint limit to be able to mint rOUSG
        ousgInstantManager.setInstantMintLimit(10_000_000e6);
    deal(address(USDC), alice, 8873775455486);
    vm.startPrank(alice);
    USDC.approve(address(ousgInstantManager), 8873775455486);
    ousgInstantManager.mintRebasingOUSG(8873775455486);
        vm.stopPrank();

    vm.startPrank(alice);
     rOUSGToken.transfer(bob,383116225977);
     uint sumOfBalanceBefore = rOUSGToken.balanceOf(alice)+rOUSGToken.balanceOf(bob);
     rOUSGToken.transferShares(bob, 1); //alice transfer 1 shares
       vm.stopPrank();
            uint256 sumOfBalanceAfter = rOUSGToken.balanceOf(alice)+rOUSGToken.balanceOf(bob);

     assert(sumOfBalanceAfter>sumOfBalanceBefore);
}

just copy past it in the rOUSG.t.sol file and setUp a fork environment.

Tools Used

Echidna

Recommended Mitigation Steps

In think the problem is that the system is too permissive about the sum of shares that you can transfer. I recommend to have a minimum amount for transferring shares .

Assessed type

MEV

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as insufficient quality report

3docSec commented 6 months ago

Inconsequential: balanceOf has few wei's fluctuations to rounding, but the actual accounting (rOUSGToken.sharesOf(alice) + rOUSGToken.sharesOf(bob);) is consistent

c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Insufficient proof