Ondo Finance - yotov721's results

Lines of code

https://github.com/code-423n4/2024-03-ondo-finance/blob/main/contracts/ousg/rOUSG.sol#L586-L606 https://github.com/code-423n4/2024-03-ondo-finance/blob/78779c30bebfd46e6f416b03066c55d587e8b30b/contracts/ousg/rOUSG.sol#L627

Vulnerability details

Impact

rOUSG is a rebasing token, meaning the amount a user owns changes. This is done by representing the tokens own by a formula of price and shares. A privileged user (admin/BURNER_ROLE) has the ability to burn any amount of shares from any account.
The contract also has a blacklist of sorts - a user needs to be KYC'd in order to make transfers of rOUSG. This is achieved by adding check in _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(_getKYCStatus(msg.sender), "rOUSG: 'sender' address not KYC'd");
    }

    if (from != address(0)) {
      // If not minting
      require(_getKYCStatus(from), "rOUSG: 'from' address not KYC'd");
    }

    if (to != address(0)) {
      // If not burning
      require(_getKYCStatus(to), "rOUSG: 'to' address not KYC'd");
    }
  }

However the admin can NOT burn tokens from a user's account if the user is/gets blacklisted as the transaction will revert and the tokens won't be burned.

Tools Used

Manual review

Recommended Mitigation Steps

Add the following change to let admin burn tokens even if the user is blacklisted. This additionally checks if the msg.sender is not an admin burning shares

  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(_getKYCStatus(msg.sender), "rOUSG: 'sender' address not KYC'd");
    }

+    if (from != address(0) && !(hasRole(BURNER_ROLE, msg.sender) && to == address(0))) {
-    if (from != address(0)) {
      // If not minting
      require(_getKYCStatus(from), "rOUSG: 'from' address not KYC'd");
    }

    if (to != address(0)) {
      // If not burning
      require(_getKYCStatus(to), "rOUSG: 'to' address not KYC'd");
    }
  }

Assessed type

Invalid Validation