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

5 stars 6 forks source link

Missing Declaration of Return Value from rousg.transferFrom #302

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/ousgInstantManager.sol#L362-L385 https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L276-L387

Vulnerability details

Impact

The redeemRebasingOUSG function does not declare or capture the return value from rousg.transferFrom, potentially leading to unintended consequences.

The rousg.transferFrom function transfers tokens but does not check or capture the return value. If the transfer fails (e.g., due to insufficient allowance), the contract will not be aware of it.

Proof of Concept

The vulnerability arises from the following aspect of the code:

function redeemRebasingOUSG(
    uint256 rousgAmountIn
  )
    external
    override
    nonReentrant
    whenRedeemNotPaused
    returns (uint256 usdcAmountOut)
  {
    require(
      rousg.allowance(msg.sender, address(this)) >= rousgAmountIn,
      "OUSGInstantManager::redeemRebasingOUSG: Insufficient allowance"
    );
    rousg.transferFrom(msg.sender, address(this), rousgAmountIn);
    rousg.unwrap(rousgAmountIn);
    uint256 ousgAmountIn = rousg.getSharesByROUSG(rousgAmountIn) /
      OUSG_TO_ROUSG_SHARES_MULTIPLIER;

this is the tranferFrom function from the rousg token contract.

 function transferFrom(
    address _sender,
    address _recipient,
    uint256 _amount
  ) public returns (bool) {
    uint256 currentAllowance = allowances[_sender][msg.sender];
    require(currentAllowance >= _amount, "TRANSFER_AMOUNT_EXCEEDS_ALLOWANCE");

    _transfer(_sender, _recipient, _amount);
    _approve(_sender, msg.sender, currentAllowance - _amount);
    return true;
  }

Tools Used

Manual review

Recommended Mitigation Steps

Capture the return value from rousg.transferFrom and handle any potential errors (e.g., revert the transaction if the transfer fails).

Assessed type

Library

c4-pre-sort commented 6 months ago

0xRobocop marked the issue as duplicate of #180

c4-judge commented 6 months ago

3docSec marked the issue as unsatisfactory: Out of scope