Unauthorized Token Transfer Due To Bad Approval Mechanism

[c4-submissions]

Lines of code

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

Vulnerability details

Impact

The identified issue pertains to the token transfer mechanism within a smart contract. Specifically, the contract allows a user to approve themselves for a certain amount of tokens without actually deducting this amount from their balance. Subsequently, when the user (or an approved spender) initiates a transfer, the contract deducts the allowance but fails to check and deduct the actual balance of the sender. This flaw allows any user to effectively mint tokens out of thin air, leading to a severe security vulnerability.

Proof of Concept

function approve(address _spender, uint256 _amount) public returns (bool) {
_approve(msg.sender, _spender, _amount);
return true;
}

an attacker calls approve as he sets _spender as his address

function _approve(
address _owner,
address _spender,
uint256 _amount
) internal whenNotPaused {
require(_owner != address(0), "APPROVE_FROM_ZERO_ADDRESS");
require(_spender != address(0), "APPROVE_TO_ZERO_ADDRESS");

allowances[_owner][_spender] = _amount;
emit Approval(_owner, _spender, _amount);
}

thus allowances[attacker][attacker] = _amount;
but _amount is not deducted from _owner's balance

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;
}

when he calls transferFrom currentAllowance would be allowances[attacker][attacker] which is _amount,
the _amount s transfered to _recipient, which can be his address as well,
_approve deduct the _amount from the allowance, means allowances[attacker][attacker] = 0,
while it was not deducted from _owner's balance, thus the _amount was sent from the contract's balance.

Tools Used

Manual review

Recommended Mitigation Steps

Modify the transferFrom function to not only check the allowance but also ensure that the sender has a sufficient balance. Deduct the transferred amount from the sender's balance.

Assessed type

Token-Transfer

[raymondfam]

It's checked here:

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

    uint256 currentSenderShares = shares[_sender];
    require(
      _sharesAmount <= currentSenderShares,
      "TRANSFER_AMOUNT_EXCEEDS_BALANCE"
    );

[c4-pre-sort]

raymondfam marked the issue as low quality report

[c4-pre-sort]

raymondfam marked the issue as primary issue

[c4-pre-sort]

raymondfam marked the issue as duplicate of #102

[c4-judge]

kirk-baird marked the issue as unsatisfactory:
Invalid