rUSDY::burn function: USDY tokens are refunded to the wrong address

[code423n4]

Lines of code

https://github.com/code-423n4/2023-09-ondo/blob/47d34d6d4a5303af5f46e907ac2292e6a7745f6c/contracts/usdy/rUSDY.sol#L672-L683

Vulnerability details

Impact

• In rUSDY contract: the burner role can burn rUSDY tokens (shares) from any user for any reason.

• The user is supposed to lose his rUSDY tokens while refunded the equivalent amount of USDY tokens.

• But burn function refunds the burner (msg.sender) with the USDY tokens instead of the intended user.

Proof of Concept

burn function

File: 2023-09-ondo/contracts/usdy/rUSDY.sol
Line 672-683:
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);
}

Tools Used

Manual Testing.

Recommended Mitigation Steps

In rUSDY::burn function: USDY amount to be refunded to the account thats has it's rUSDY tokens burnt:

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

    _burnShares(_account, sharesAmount);

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

    emit TokensBurnt(_account, _amount);
  }

Assessed type

Context

[raymondfam]

A valid point. But will leave it to the sponsor whether or not this is intended.

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as sufficient quality report

[c4-pre-sort]

raymondfam marked the issue as high quality report

[c4-sponsor]

ypatil12 (sponsor) disputed

[c4-sponsor]

ypatil12 marked the issue as disagree with severity

[ypatil12]

This is by design as a permissioned token. If a user who mints rUSDY becomes sanctioned, we would have to burn the user's rUSDY and seize there USDY. The user with the BURNER_ROLE is a trusted party.

[kirk-baird]

Given this is a clear design choice and is behaving as intended I'm going to reduce this issue to QA since it does provide the admin with a method of draining USDY from the protocol.

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[c4-judge]

kirk-baird marked the issue as grade-c

[c4-judge]

This previously downgraded issue has been upgraded by kirk-baird

[c4-judge]

kirk-baird changed the severity to QA (Quality Assurance)

[c4-judge]

kirk-baird marked the issue as grade-b