Critical Vulnerability in ONDO Contract: Addressing the Rug Pull Risk in the Burn Function

[c4-bot-6]

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/rOUSG.sol#L618-L640

Vulnerability details

Crypto is the land of rug pulls, and even the most credible players can flee with investors’ money. Recall SBF, who defrauded FTX users and laundered their funds. Also, the Africrypt exchange founders stole almost $4 billion in one of the most extraordinary rugs in crypto.

Impact

As stated by the contract in the docs "We would be open to suggestions on ways we can further restrain the setters without decreasing operational overhead, convenience, or flexibility of the code." The impact of the identified vulnerability in the ONDO contract's burn function is significant and exposes users to the risk of losing their OUSG tokens. The current implementation allows the OUSG tokens, which belong to _account, to be transferred to msg.sender, which is the address with the burners role. This creates a potential for a rug pull, where the contract owner or administrator could maliciously withdraw the OUSG tokens. As long as the function exist and can send users money to the caller and not te actual owner, Even if the burners role is assigned to only trusted accounts, the risk of a rug pull remains high, jeopardizing the security and trustworthiness of the ONDO contract and its ecosystem.

Proof of Concept

1.The function burns the shares on an account holding rOUSG (address _account, uint256 _amount) 2.BURNS the share of that account _burnShares(_account, ousgSharesAmount); But transfer of the equivalent OUSG is sent to the caller of the burn function is not the account holder. Thus this function allows a burner to pass in any account burn the shares associated with it an get the OUSG token value .

ousg.transfer( msg.sender, ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER ); 3.Since wrap rOUSG is still OUSG but wrapped the token should be sent back to the account instead and not msg.sender

Code Snippet Before Mitigation

/**
 * @notice Admin burn function to burn rOUSG tokens from any account
 * @param _account The account to burn tokens from
 * @param _amount  The amount of rOUSG tokens to burn
 * @dev Transfers burned shares (OUSG) to `msg.sender`
 */
function burn(
  address _account,
  uint256 _amount
) external onlyRole(BURNER_ROLE) {
  uint256 ousgSharesAmount = getSharesByROUSG(_amount);
  if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
    revert UnwrapTooSmall();

  _burnShares(_account, ousgSharesAmount);

  ousg.transfer(
    msg.sender,
    ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
  );
  emit Transfer(address(0), msg.sender, getROUSGByShares(ousgSharesAmount));
  emit TransferShares(_account, address(0), ousgSharesAmount);
}

Tools Used

Manual code analysis was used to identify and analyze the vulnerability in the ONDO contract.

Recommended Mitigation Steps

To mitigate the identified vulnerability and enhance the security of the ONDO contract, the following steps are recommended:

1. Update the ONDO contract code to implement the proposed mitigation strategy by modifying the burn function to return the OUSG tokens to the _account that owns them after the burn operation.

``/**
 * @notice Admin burn function to burn rOUSG tokens from any account
 * @param _account The account to burn tokens from
 * @param _amount  The amount of rOUSG tokens to burn
 * @dev Transfers burned shares (OUSG) to the burned account
 */
function burn(
  address _account,
  uint256 _amount
) external onlyRole(BURNER_ROLE) {
  require(rOUSG.balanceOf(_account) >= _amount, "Insufficient rOUSG balance");
  require(_amount <= rOUSG.balanceOf(_account), "Burn amount exceeds rOUSG balance");
  require(OUSG_TO_ROUSG_SHARES_MULTIPLIER != 0, "Multiplier should not be zero");

  uint256 ousgSharesAmount = getSharesByROUSG(_amount);
  if (ousgSharesAmount < OUSG_TO_ROUSG_SHARES_MULTIPLIER)
    revert UnwrapTooSmall();

  _burnShares(_account, ousgSharesAmount);

  ousg.transfer(
    _account,  // Transfer to the burner's account (which can be a WETH account)
    ousgSharesAmount / OUSG_TO_ROUSG_SHARES_MULTIPLIER
  );

  emit Transfer(address(0), _account, getROUSGByShares(ousgSharesAmount));
  emit TransferShares(_account, address(0), ousgSharesAmount);
}

By proactively addressing this vulnerability and implementing the recommended mitigation steps, the ONDO contract can enhance its security and protect its users from the risk of a rug pull, thereby strengthening its credibility and reliability in the decentralized finance (DeFi) space.

Assessed type

Token-Transfer

[c4-pre-sort]

0xRobocop marked the issue as duplicate of #22

[c4-judge]

3docSec marked the issue as unsatisfactory: Out of scope