Ondo Finance - 0xAsen's results

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/main/contracts/usdy/rUSDY.sol#L642

Vulnerability details

Impact

The function burn is made so the admin can burn rUSDY tokens from any account - this is stated in the comments. However, the admin can't burn tokens if the account from which he's trying to burn tokens is blocklisted/sanctioned/not on the allowlist.

Proof of Concept

Let's check the burn function which calls the internal _burnShares function:

function burn(
    address _account,
    uint256 _amount
  ) external onlyRole(BURNER_ROLE) {
    uint256 sharesAmount = getSharesByRUSDY(_amount);

    _burnShares(_account, sharesAmount);

    usdy.transfer(msg.sender, sharesAmount / BPS_DENOMINATOR);

    emit TokensBurnt(_account, _amount);
  }

  function _burnShares(
    address _account,
    uint256 _sharesAmount
  ) internal whenNotPaused returns (uint256) {
    require(_account != address(0), "BURN_FROM_THE_ZERO_ADDRESS");

    _beforeTokenTransfer(_account, address(0), _sharesAmount); <--

    uint256 accountShares = shares[_account];
    require(_sharesAmount <= accountShares, "BURN_AMOUNT_EXCEEDS_BALANCE");

    uint256 preRebaseTokenAmount = getRUSDYByShares(_sharesAmount);

    totalShares -= _sharesAmount;

    shares[_account] = accountShares - _sharesAmount;

    uint256 postRebaseTokenAmount = getRUSDYByShares(_sharesAmount);

    return totalShares;

We can see that it calls _beforeTokenTransfer(_account, address(0), _sharesAmount) Here is the code of _beforeTokenTransfer:

function _beforeTokenTransfer(
    address from,
    address to,
    uint256
  ) internal view {
    // Check constraints when `transferFrom` is called to facliitate
    // a transfer between two parties that are not `from` or `to`.
    if (from != msg.sender && to != msg.sender) {
      require(!_isBlocked(msg.sender), "rUSDY: 'sender' address blocked");
      require(!_isSanctioned(msg.sender), "rUSDY: 'sender' address sanctioned");
      require(
        _isAllowed(msg.sender),
        "rUSDY: 'sender' address not on allowlist"
      );
    }

    if (from != address(0)) { <--
      // If not minting
      require(!_isBlocked(from), "rUSDY: 'from' address blocked");
      require(!_isSanctioned(from), "rUSDY: 'from' address sanctioned");
      require(_isAllowed(from), "rUSDY: 'from' address not on allowlist");
    }

    if (to != address(0)) {
      // If not burning
      require(!_isBlocked(to), "rUSDY: 'to' address blocked");
      require(!_isSanctioned(to), "rUSDY: 'to' address sanctioned");
      require(_isAllowed(to), "rUSDY: 'to' address not on allowlist");
    }
  }

In our case, the from would be the account from which we're burning tokens so it'll enter in the 2nd if - if (from != address(0)). But given that the account is blocked/sanctioned/not on the allowlist, the transaction will revert and the tokens won't be burned.

Given that there are separate roles for burning and managing the block/sanctions/allowed lists - BURNER_ROLE and LIST_CONFIGURER_ROLE, it is very possible that such a scenario would occur.

In that case, the Burner would have to ask the List Configurer to update the lists, so the Burner can burn the tokens, and then the List Configurer should update the lists again. However, in that case, you're risking that the blocked user manages to transfer their funds while performing these operations.

Tools Used

Manual review

Recommended Mitigation Steps

Organize the logic of the function better. For example, you can make the 2nd if to be: if (from != address(0) && to != address(0)), that way we'll not enter the if when burning tokens, and we'll be able to burn tokens from blocked accounts.

Assessed type

Invalid Validation

1. Summary

1.1 Description

The main assets in the protocol are USDY and rUSDY which is actually not a real token but it is kept as an internal balance in the rUSDY.sol contract. rUSDY is interest-bearing stablecoin users receive in return for wrapping their USDY. Each 1 rUSDY token is worth 1 dollar and its yield is calculated based on the time you wrapped your USDY and the price of USDY. The yield is implemented by rebasing the supply of rUSDY accordingly when the price of USDY increases.

1.2 Contracts in scope and how to they interact with each other

The contracts in scope were: SourceBridge.sol - Source chain bridge contract for USDY DestinationBridge.sol - Destination chain bridge contract for USDY

rUSDY.sol - rebasing USDY token contract rUSDYFactory.sol - used for deploying rUSDY

RWADynamicOracle.sol - custom oracle used by rUSDY to get the price of USDY and rebase accordingly

Interactions: SourceBridge.sol interacts with DestinationBridge.sol through the AxelarGateway enabling users to bridge USDY from one chain to another.

rUSDYFactory.sol is used to deploy rUSDY.sol and rUSDY.sol interacts with RWADynamicOracle.sol to receive the price of USDY and measure accrued yield by rebasing.

1.3 User flow

For rUSDY.sol: A user has USDY tokens —> wraps them with the wrap function and receives rUSDY i.e. “shares” in return. The shares amount he receives is the USDYamount * 10_000 —> unwraps them and receives his USDY back including the accrued yield for the time he held rUSDY A user also has the functionality to transfer and approve rUSDY tokens.

For bridging: A user has USDY on Source chain —> calls burnAndCallAxelar where he passes amount and destinationChain for parameters, with some native tokens for gas —> AxelarGateway handles the paying of gas and calling the correct contract and calls the execute function on the DestinationBridge contract —> from there a txnHash is assigned to the transaction and the transaction is minted if a certain number of approvers have approved it —> transaction is minted and the user receives his tokens on the Destination Chain.

Main roles

Except for the users, there are a few governance roles that are responsible for different parts of the protocol: DEFAULT_ADMIN_ROLE, USDY_MANAGER_ROLE, PAUSER_ROLE, MINTER_ROLE, BURNER_ROLE, LIST_CONFIGURER_ROLE

2. Approach taken

• Started with reading the documentation
• Read the contracts, then the documentation again, noting everything that looks suspicious
• Deep dive into the contracts

Suggestions for improvement

For important suggestions for improvement, I'd recommend implementing a real oracle for determining the price of the USDY. Using a custom oracle like the one you implemented not only introduces a lot of Centralization risks as the owner can set the price to whatever price he wants, but also introduces external attack vectors as one of the findings I submitted.

Time spent:

15 hours